Skip to content

Commit

Permalink
Refactor and write tests against the internal services
Browse files Browse the repository at this point in the history
  • Loading branch information
amarsan committed Nov 14, 2024
1 parent 3d8f33d commit d3b3e25
Show file tree
Hide file tree
Showing 4 changed files with 184 additions and 121 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
import org.ohdsi.webapi.achilles.domain.AchillesCacheEntity;
import org.ohdsi.webapi.achilles.repository.AchillesCacheRepository;
import org.ohdsi.webapi.source.Source;
import org.ohdsi.webapi.source.SourceRepository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Propagation;
Expand All @@ -22,108 +24,118 @@

@Service
public class AchillesCacheService {
private static final Logger LOG = LoggerFactory.getLogger(AchillesCacheService.class);

private final AchillesCacheRepository cacheRepository;

private final ObjectMapper objectMapper;

@Value("${spring.jpa.properties.hibernate.jdbc.batch_size}")
private int batchSize;

public AchillesCacheService(AchillesCacheRepository cacheRepository,
ObjectMapper objectMapper) {
this.cacheRepository = cacheRepository;
this.objectMapper = objectMapper;
}

@Transactional(readOnly = true)
public AchillesCacheEntity getCache(Source source, String cacheName) {
return cacheRepository.findBySourceAndCacheName(source, cacheName);
private static final Logger LOG = LoggerFactory.getLogger(AchillesCacheService.class);

private final AchillesCacheRepository cacheRepository;

private final ObjectMapper objectMapper;

@Value("${spring.jpa.properties.hibernate.jdbc.batch_size}")
private int batchSize;

@Autowired
private SourceRepository sourceRepository;

public AchillesCacheService(AchillesCacheRepository cacheRepository,
ObjectMapper objectMapper) {
this.cacheRepository = cacheRepository;
this.objectMapper = objectMapper;
}

@Transactional(readOnly = true)
public AchillesCacheEntity getCache(Source source, String cacheName) {
return cacheRepository.findBySourceAndCacheName(source, cacheName);
}

@Transactional(readOnly = true)
public List<AchillesCacheEntity> findBySourceAndNames(Source source, List<String> names) {
return cacheRepository.findBySourceAndNames(source, names);
}

@Transactional(propagation = Propagation.REQUIRES_NEW)
public AchillesCacheEntity createCache(Source source, String cacheName, Object result)
throws JsonProcessingException {
AchillesCacheEntity cacheEntity = getCache(source, cacheName);
String cache = objectMapper.writeValueAsString(result);
if (Objects.nonNull(cacheEntity)) {
cacheEntity.setCache(cache);
} else {
cacheEntity = new AchillesCacheEntity();
cacheEntity.setSource(source);
cacheEntity.setCacheName(cacheName);
cacheEntity.setCache(cache);
}

@Transactional(readOnly = true)
public List<AchillesCacheEntity> findBySourceAndNames(Source source, List<String> names) {
return cacheRepository.findBySourceAndNames(source, names);
}

@Transactional(propagation = Propagation.REQUIRES_NEW)
public AchillesCacheEntity createCache(Source source, String cacheName, Object result) throws JsonProcessingException {
AchillesCacheEntity cacheEntity = getCache(source, cacheName);
String cache = objectMapper.writeValueAsString(result);
if (Objects.nonNull(cacheEntity)) {
cacheEntity.setCache(cache);
} else {
cacheEntity = new AchillesCacheEntity();
cacheEntity.setSource(source);
cacheEntity.setCacheName(cacheName);
cacheEntity.setCache(cache);
}
return cacheRepository.save(cacheEntity);
}

return cacheRepository.save(cacheEntity);
@Transactional(propagation = Propagation.REQUIRES_NEW)
public void saveDrilldownCacheMap(Source source, String domain, Map<Integer, ObjectNode> conceptNodes) {
if (conceptNodes.isEmpty()) { // nothing to cache
LOG.warn(
"Cannot cache drilldown reports for {}, domain {}. Check if result schema contains achilles results tables.",
source.getSourceKey(), domain);
return;
}

@Transactional(propagation = Propagation.REQUIRES_NEW)
public void saveDrilldownCacheMap(Source source, String domain, Map<Integer, ObjectNode> conceptNodes) {
if (conceptNodes.isEmpty()) { // nothing to cache
LOG.warn("Cannot cache drilldown reports for {}, domain {}. Check if result schema contains achilles results tables.",
source.getSourceKey(), domain);
return;
}

Map<String, ObjectNode> nodes = new HashMap<>(batchSize);
for (Map.Entry<Integer, ObjectNode> entry : conceptNodes.entrySet()) {
if (nodes.size() >= batchSize) {
createCacheEntities(source, nodes);
nodes.clear();
}
Integer key = entry.getKey();
String cacheName = getCacheName(domain, key);
nodes.put(cacheName, entry.getValue());
}
Map<String, ObjectNode> nodes = new HashMap<>(batchSize);
for (Map.Entry<Integer, ObjectNode> entry : conceptNodes.entrySet()) {
if (nodes.size() >= batchSize) {
createCacheEntities(source, nodes);
nodes.clear();
}
Integer key = entry.getKey();
String cacheName = getCacheName(domain, key);
nodes.put(cacheName, entry.getValue());
}

@Transactional()
public void clearCache(Source source) {
cacheRepository.deleteBySource(source);
}

private void createCacheEntities(Source source, Map<String, ObjectNode> nodes) {
List<AchillesCacheEntity> cacheEntities = getEntities(source, nodes);
cacheRepository.save(cacheEntities);
}

private List<AchillesCacheEntity> getEntities(Source source, Map<String, ObjectNode> nodes) {
List<String> cacheNames = new ArrayList<>(nodes.keySet());
List<AchillesCacheEntity> cacheEntities = findBySourceAndNames(source, cacheNames);
nodes.forEach((key, value) -> {
// check if the entity with given cache name already exists
Optional<AchillesCacheEntity> cacheEntity = cacheEntities.stream()
.filter(entity -> entity.getCacheName().equals(key))
.findAny();
try {
String newValue = objectMapper.writeValueAsString(value);
if (cacheEntity.isPresent()) {
// if cache entity already exists update its value
cacheEntity.get().setCache(newValue);
} else {
// if cache entity does not exist - create new one
AchillesCacheEntity newEntity = new AchillesCacheEntity();
newEntity.setCacheName(key);
newEntity.setSource(source);
newEntity.setCache(newValue);
cacheEntities.add(newEntity);
}
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
});
return cacheEntities;
}

private String getCacheName(String domain, int conceptId) {
return String.format("drilldown_%s_%d", domain, conceptId);
}
createCacheEntities(source, nodes);
nodes.clear();
}

@Transactional()
public void clearCache() {
List<Source> sources = sourceRepository.findAll();
sources.parallelStream().forEach(this::clearCache);
}

private void clearCache(Source source) {
cacheRepository.deleteBySource(source);
}

private void createCacheEntities(Source source, Map<String, ObjectNode> nodes) {
List<AchillesCacheEntity> cacheEntities = getEntities(source, nodes);
cacheRepository.save(cacheEntities);
}

private List<AchillesCacheEntity> getEntities(Source source, Map<String, ObjectNode> nodes) {
List<String> cacheNames = new ArrayList<>(nodes.keySet());
List<AchillesCacheEntity> cacheEntities = findBySourceAndNames(source, cacheNames);
nodes.forEach((key, value) -> {
// check if the entity with given cache name already exists
Optional<AchillesCacheEntity> cacheEntity = cacheEntities.stream()
.filter(entity -> entity.getCacheName().equals(key))
.findAny();
try {
String newValue = objectMapper.writeValueAsString(value);
if (cacheEntity.isPresent()) {
// if cache entity already exists update its value
cacheEntity.get().setCache(newValue);
} else {
// if cache entity does not exist - create new one
AchillesCacheEntity newEntity = new AchillesCacheEntity();
newEntity.setCacheName(key);
newEntity.setSource(source);
newEntity.setCache(newValue);
cacheEntities.add(newEntity);
}
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
});
return cacheEntities;
}

private String getCacheName(String domain, int conceptId) {
return String.format("drilldown_%s_%d", domain, conceptId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,6 @@ public void warm(Source source) {
}
}

@Transactional()
public void clearCache(Source source) {
cdmCacheRepository.deleteBySource(source.getSourceId());
}

public List<CDMCacheEntity> findAndCache(Source source, List<Integer> conceptIds) {
if (CollectionUtils.isEmpty(conceptIds)) {
return Collections.emptyList();
Expand All @@ -101,6 +96,16 @@ public List<CDMCacheEntity> findAndCache(Source source, List<Integer> conceptIds
return cacheEntities;
}

@Transactional()
public void clearCache() {
List<Source> sources = getSourceRepository().findAll();
sources.parallelStream().forEach(this::clearCache);
}

private void clearCache(Source source) {
cdmCacheRepository.deleteBySource(source.getSourceId());
}

private List<CDMCacheEntity> find(Source source, List<Integer> conceptIds) {
if (CollectionUtils.isEmpty(conceptIds)) {
return Collections.emptyList();
Expand Down
15 changes: 2 additions & 13 deletions src/main/java/org/ohdsi/webapi/service/CDMResultsService.java
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,8 @@ public void clearCache() {
if (!isSecured() || !isAdmin()) {
throw new ForbiddenException();
}
List<Source> sources = getSourceRepository().findAll();
sources.parallelStream().forEach(this::clearCache);
cacheService.clearCache();
cdmCacheService.clearCache();
}

/**
Expand Down Expand Up @@ -519,17 +519,6 @@ private void warmCaches(Collection<Source> sources) {
}
}

/*
* Clear cache for a single source
*/
private void clearCache(Source source) {
if (!sourceAccessor.hasAccess(source)) {
return;
}
cacheService.clearCache(source);
cdmCacheService.clearCache(source);
}

private SimpleJobBuilder createJob(String jobName, List<Step> steps) {
final SimpleJobBuilder[] stepBuilder = {null};
if (jobService.findJobByName(jobName, jobName) == null && !steps.isEmpty()) {
Expand Down
71 changes: 64 additions & 7 deletions src/test/java/org/ohdsi/webapi/test/CDMResultsServiceIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
import java.util.Map;

import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.ohdsi.circe.helper.ResourceHelper;
import org.ohdsi.sql.SqlRender;
import org.ohdsi.sql.SqlTranslate;
import org.ohdsi.webapi.achilles.service.AchillesCacheService;
import org.ohdsi.webapi.cdmresults.service.CDMCacheService;
import org.ohdsi.webapi.source.SourceRepository;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
Expand All @@ -35,6 +36,12 @@ public class CDMResultsServiceIT extends WebApiIT {
@Autowired
private SourceRepository sourceRepository;

@Autowired
private AchillesCacheService achillesService;

@Autowired
private CDMCacheService cdmCacheService;

@Before
public void init() throws Exception {
truncateTable(String.format("%s.%s", "public", "source"));
Expand Down Expand Up @@ -83,17 +90,67 @@ public void requestConceptRecordCounts_firstTime_returnsResults() {
assertEquals(103, counts.get(3).intValue());
}

// This is ignored right now because the clearCache method requires that security be set up and I'm not sure how to do that in this context
@Ignore
@Test
public void clearCache_nothingInCache_returns() {
public void achillesService_clearCache_nothingInCache_doesNothing() {

// Arrange

// Act
final ResponseEntity<String> entity = getRestTemplate().postForEntity(this.clearCacheEndpoint, null, String.class);
achillesService.clearCache();

// Assert
assertOK(entity);
String sql = "SELECT COUNT(*) FROM achilles_cache";
Integer count = jdbcTemplate.queryForObject(sql, Integer.class);
assertEquals(0, count.intValue());
}

@Test
public void achillesService_clearCache_somethingInCache_clearsAllRowsForSource() {

// Arrange
String insertSqlRow1 = "INSERT INTO achilles_cache (id, source_id, cache_name, cache) VALUES (1, 1, 'cache1', 'cache1')";
jdbcTemplate.execute(insertSqlRow1);
String insertSqlRow2 = "INSERT INTO achilles_cache (id, source_id, cache_name, cache) VALUES (2, 1, 'cache2', 'cache2')";
jdbcTemplate.execute(insertSqlRow2);

// Act
achillesService.clearCache();

// Assert
String sql = "SELECT COUNT(*) FROM achilles_cache";
Integer count = jdbcTemplate.queryForObject(sql, Integer.class);
assertEquals(0, count.intValue());
}

@Test
public void cdmCacheService_clearCache_nothingInCache_doesNothing() {

// Arrange

// Act
cdmCacheService.clearCache();

// Assert
String sql = "SELECT COUNT(*) FROM cdm_cache";
Integer count = jdbcTemplate.queryForObject(sql, Integer.class);
assertEquals(0, count.intValue());
}

@Test
public void cdmCacheService_clearCache_somethingInCache_clearsAllRowsForSource() {

// Arrange
String insertSqlRow1 = "INSERT INTO cdm_cache (id, concept_id, source_id, record_count, descendant_record_count, person_count, descendant_person_count) VALUES (1, 1, 1, 100, 101, 102, 103)";
jdbcTemplate.execute(insertSqlRow1);
String insertSqlRow2 = "INSERT INTO cdm_cache (id, concept_id, source_id, record_count, descendant_record_count, person_count, descendant_person_count) VALUES (2, 2, 1, 200, 201, 202, 203)";
jdbcTemplate.execute(insertSqlRow2);

// Act
cdmCacheService.clearCache();

// Assert
String sql = "SELECT COUNT(*) FROM cdm_cache";
Integer count = jdbcTemplate.queryForObject(sql, Integer.class);
assertEquals(0, count.intValue());
}
}
}

0 comments on commit d3b3e25

Please sign in to comment.