-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implemented fallback mechanism to TRAX v2 endpoints that obtain data… #394
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,12 +3,16 @@ | |
import ca.bc.gov.educ.api.trax.constant.CacheKey; | ||
import ca.bc.gov.educ.api.trax.exception.ServiceException; | ||
import ca.bc.gov.educ.api.trax.model.dto.institute.District; | ||
|
||
import ca.bc.gov.educ.api.trax.model.dto.institute.SchoolDetail; | ||
import ca.bc.gov.educ.api.trax.model.entity.institute.DistrictEntity; | ||
|
||
import ca.bc.gov.educ.api.trax.model.transformer.institute.DistrictTransformer; | ||
import ca.bc.gov.educ.api.trax.repository.redis.DistrictRedisRepository; | ||
import ca.bc.gov.educ.api.trax.service.RESTService; | ||
import ca.bc.gov.educ.api.trax.util.EducGradTraxApiConstants; | ||
import ca.bc.gov.educ.api.trax.util.SearchUtil; | ||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.beans.factory.annotation.Qualifier; | ||
|
@@ -17,7 +21,9 @@ | |
import org.springframework.web.reactive.function.client.WebClientResponseException; | ||
|
||
import java.util.ArrayList; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Optional; | ||
|
||
@Slf4j | ||
@Service("InstituteDistrictService") | ||
|
@@ -59,9 +65,43 @@ public void loadDistrictsIntoRedisCache(List<District> districts) { | |
log.info(String.format("%s Districts Loaded into cache.", districts.size())); | ||
} | ||
|
||
public List <District> getDistrictsBySearchCriteriaFromInstituteApi(String key, String value) { | ||
try { | ||
log.debug("****Before Calling Institute API"); | ||
HashMap<String, String> params; | ||
HashMap<String, String> searchInput = new HashMap<>(); | ||
searchInput.put(key, value); | ||
|
||
try { | ||
params = SearchUtil.searchStringsToHTTPParams(searchInput); | ||
} catch (JsonProcessingException e) { | ||
throw new RuntimeException(e); | ||
} | ||
|
||
|
||
List <DistrictEntity> districtEntities = this.restService.get(constants.getDistrictsPaginated(),params, | ||
List.class, webClient); | ||
return districtTransformer.transformToDTO(districtEntities); | ||
} catch (WebClientResponseException e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exception and WebClientException are caught in the RESTService.get method already which throws a ServiceException. Either catch the service exception and deal with it here or pass it up to the controller level where it can return an error to the caller. Also, be aware that you can utilize the RestErrorHandler class to capture thrown errors and deal with them at the controller level There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed, created a new PR #395 |
||
log.warn("Error getting District By search Criteria from Institute API"); | ||
} catch (Exception e) { | ||
log.error(String.format("Error while calling Institute api: %s", e.getMessage())); | ||
} | ||
return null; | ||
} | ||
|
||
public List<District> getDistrictsFromRedisCache() { | ||
log.debug("**** Getting districts from Redis Cache."); | ||
return districtTransformer.transformToDTO(districtRedisRepository.findAll()); | ||
Iterable<DistrictEntity> districtEntities= districtRedisRepository.findAll(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this. We don't want to trigger a wholesale refresh of the cache. We are only interested in specific districts, not for get all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed, created a new PR #395 |
||
if ( (!districtEntities.iterator().hasNext())){ | ||
log.debug("Get District from Redis Cache returned empty"); | ||
List<District> districts = this.getDistrictsFromInstituteApi(); | ||
if ((districts != null) && (!districts .isEmpty())) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comment on getDistrictByIdFromRedisCache method regarding null checks on Optional There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. N/A this returns Iterable not optional |
||
this.loadDistrictsIntoRedisCache(districts); | ||
return districts; | ||
} | ||
} | ||
return districtTransformer.transformToDTO(districtEntities); | ||
} | ||
|
||
public void initializeDistrictCache(boolean force) { | ||
|
@@ -70,14 +110,36 @@ public void initializeDistrictCache(boolean force) { | |
|
||
public District getDistrictByDistNoFromRedisCache(String districtNumber) { | ||
log.debug("**** Getting district by district no. from Redis Cache."); | ||
return districtTransformer.transformToDTO(districtRedisRepository.findByDistrictNumber(districtNumber)); | ||
DistrictEntity districtEntity = districtRedisRepository.findByDistrictNumber(districtNumber); | ||
if ( districtEntity == null){ | ||
log.debug("Getting district by district no from Redis Cache returned empty"); | ||
List<District> districts = this.getDistrictsBySearchCriteriaFromInstituteApi("districtNumber",districtNumber ); | ||
if ((districts != null) &&(!districts.isEmpty())){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comment on getDistrictByIdFromRedisCache method regarding null checks on Optional There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed created a new PR #395 |
||
this.loadDistrictsIntoRedisCache(districts); | ||
return districts.get(0); | ||
} | ||
} | ||
return districtTransformer.transformToDTO(districtEntity); | ||
} | ||
|
||
public District getDistrictByIdFromRedisCache(String districtId) { | ||
log.debug("**** Getting district by ID from Redis Cache."); | ||
return districtTransformer.transformToDTO(districtRedisRepository.findById(districtId)); | ||
Optional<DistrictEntity> districtEntity = districtRedisRepository.findById(districtId); | ||
if((districtEntity == null) || (districtEntity.isEmpty())) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not need to check for null or isEmpty. You can use orElseGet on Optional class. Something like this:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed, created a new PR #395 |
||
log.debug("Getting district by ID from Redis Cache returned empty"); | ||
District district = this.getDistrictByIdFromInstituteApi(districtId); | ||
if (district!=null) { | ||
this.loadDistrictsIntoRedisCache(List.of(district)); | ||
return district; | ||
} | ||
|
||
} | ||
return districtTransformer.transformToDTO(districtEntity); | ||
} | ||
|
||
|
||
|
||
|
||
public List<District> getDistrictsBySchoolCategoryCode(String schoolCategoryCode) { | ||
List<SchoolDetail> schoolDetails; | ||
|
||
|
@@ -93,6 +155,20 @@ public List<District> getDistrictsBySchoolCategoryCode(String schoolCategoryCode | |
return districts; | ||
} | ||
|
||
public District getDistrictByIdFromInstituteApi(String districtId) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again no need to catch Exceptions here. You can throw a ServiceException like the updateDistrictCache method below and also expect a District object returned instead of an Optional. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed created a new PR #395. The method already returns District object |
||
try { | ||
log.debug("****Before Calling Institute API"); | ||
Optional<DistrictEntity> districtEntity = this.restService.get(String.format(constants.getGetDistrictFromInstituteApiUrl(), districtId), | ||
Optional.class, webClient); | ||
return districtTransformer.transformToDTO(districtEntity); | ||
} catch (WebClientResponseException e) { | ||
log.warn("Error getting District"); | ||
} catch (Exception e) { | ||
log.error(String.format("Error while calling Institute api: %s", e.getMessage())); | ||
} | ||
return null; | ||
} | ||
|
||
/** | ||
* Updates the district details in the cache | ||
* based on schoolId | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,15 +12,18 @@ | |
import ca.bc.gov.educ.api.trax.repository.redis.SchoolRedisRepository; | ||
import ca.bc.gov.educ.api.trax.service.RESTService; | ||
import ca.bc.gov.educ.api.trax.util.EducGradTraxApiConstants; | ||
|
||
import ca.bc.gov.educ.api.trax.util.SearchUtil; | ||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.beans.factory.annotation.Qualifier; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.web.reactive.function.client.WebClient; | ||
import org.springframework.web.reactive.function.client.WebClientResponseException; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
import java.util.*; | ||
|
||
@Slf4j | ||
@Service("InstituteSchoolService") | ||
|
@@ -66,12 +69,33 @@ public void loadSchoolsIntoRedisCache(List<ca.bc.gov.educ.api.trax.model.dto.ins | |
|
||
public List<School> getSchoolsFromRedisCache() { | ||
log.debug("**** Getting schools from Redis Cache."); | ||
return schoolTransformer.transformToDTO(schoolRedisRepository.findAll()); | ||
Iterable<SchoolEntity> schoolEntities = schoolRedisRepository.findAll(); | ||
if ( (!schoolEntities.iterator().hasNext())){ | ||
log.debug("Get Schools from Redis Cache returned empty"); | ||
List<School> schools = getSchoolsFromInstituteApi(); | ||
if ((schools != null) && (!schools.isEmpty())) { | ||
loadSchoolsIntoRedisCache(schools); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove code block here. Cache is initialized on load by a single pod using a cache key. The only reason the cache returns empty is the edge case when one pod is loading cache while another gets the request at the controller. Just return the schools List from Institute and we should be good. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed, created a new PR #395 |
||
return schools; | ||
} | ||
} | ||
return schoolTransformer.transformToDTO(schoolEntities); | ||
} | ||
|
||
public School getSchoolByMincodeFromRedisCache(String mincode) { | ||
log.debug("Get School by Mincode from Redis Cache"); | ||
return schoolTransformer.transformToDTO(schoolRedisRepository.findByMincode(mincode)); | ||
SchoolEntity schoolEntity = schoolRedisRepository.findByMincode(mincode); | ||
if (schoolEntity == null) { | ||
log.debug("Get School by Mincode from Redis Cache returned null"); | ||
List<School> schools = getSchoolsBySearchCriteriaFromInstituteApi("mincode", mincode); | ||
if ((schools != null) && (!schools.isEmpty())) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comment on DistrictService.getDistrictByIdFromRedisCache method regarding null checks on Optional There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed created a new PR #395 |
||
School school = schools.get(0); | ||
if (school != null) { | ||
loadSchoolsIntoRedisCache(List.of(school)); | ||
return school; | ||
} | ||
} | ||
} | ||
return schoolTransformer.transformToDTO(schoolEntity); | ||
} | ||
|
||
public boolean checkIfSchoolExists(String minCode) { | ||
|
@@ -97,6 +121,33 @@ public SchoolDetail getSchoolDetailByIdFromInstituteApi(String schoolId) { | |
return null; | ||
} | ||
|
||
|
||
|
||
public List <School> getSchoolsBySearchCriteriaFromInstituteApi(String key, String value) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comments on getDistrictsBySearchCriteriaFromInstituteApi. Please change accordingly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed created a new PR #395 |
||
try { | ||
log.debug("****Before Calling Institute API"); | ||
HashMap<String, String> params; | ||
HashMap<String, String> searchInput = new HashMap<>(); | ||
searchInput.put(key, value); | ||
|
||
try { | ||
params = SearchUtil.searchStringsToHTTPParams(searchInput); | ||
} catch (JsonProcessingException e) { | ||
throw new RuntimeException(e); | ||
} | ||
|
||
|
||
List <SchoolEntity> schoolEntities = this.restService.get(constants.getSchoolsPaginated(),params, | ||
List.class, webClient); | ||
return schoolTransformer.transformToDTO(schoolEntities); | ||
} catch (WebClientResponseException e) { | ||
log.warn("Error getting School By search Criteria from Institute API"); | ||
} catch (Exception e) { | ||
log.error(String.format("Error while calling Institute api: %s", e.getMessage())); | ||
} | ||
return null; | ||
} | ||
|
||
public List<SchoolDetail> getSchoolDetailsFromInstituteApi() { | ||
|
||
List<School> schools = getSchoolsFromRedisCache(); | ||
|
@@ -110,17 +161,40 @@ public List<SchoolDetail> getSchoolDetailsFromInstituteApi() { | |
public void loadSchoolDetailsIntoRedisCache(List<SchoolDetail> schoolDetails) { | ||
schoolDetailRedisRepository | ||
.saveAll(schoolDetailTransformer.transformToEntity(schoolDetails)); | ||
log.info(String.format("%s School Details Loaded into cache.", schoolDetails.size())); | ||
log.debug(String.format("%s School Details Loaded into cache.", schoolDetails.size())); | ||
} | ||
|
||
public List<SchoolDetail> getSchoolDetailsFromRedisCache() { | ||
log.debug("**** Getting school Details from Redis Cache."); | ||
return schoolDetailTransformer.transformToDTO(schoolDetailRedisRepository.findAll()); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this code block. We don't want to trigger a wholesale refresh of cache at this point. We are only interested in calling back to institute for individual institutes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed created a new PR #395 |
||
Iterable<SchoolDetailEntity> schoolDetailEntities = schoolDetailRedisRepository.findAll(); | ||
if ( (!schoolDetailEntities.iterator().hasNext())){ | ||
log.debug("Get Schools details from Redis Cache returned empty"); | ||
List<SchoolDetail> schoolDetails = this.getSchoolDetailsFromInstituteApi(); | ||
if ((schoolDetails != null) && (!schoolDetails .isEmpty())) { | ||
loadSchoolDetailsIntoRedisCache(schoolDetails); | ||
return schoolDetails; | ||
} | ||
} | ||
return schoolDetailTransformer.transformToDTO(schoolDetailEntities); | ||
} | ||
|
||
public SchoolDetail getSchoolDetailByMincodeFromRedisCache(String mincode) { | ||
log.debug("**** Getting school Details By Mincode from Redis Cache."); | ||
return schoolDetailTransformer.transformToDTO(schoolDetailRedisRepository.findByMincode(mincode)); | ||
SchoolDetailEntity schoolDetailEntity = schoolDetailRedisRepository.findByMincode(mincode); | ||
if ( schoolDetailEntity == null){ | ||
log.debug("Get Schools details by mincode from Redis Cache returned empty"); | ||
List<School> schools = this.getSchoolsBySearchCriteriaFromInstituteApi("mincode", mincode); | ||
if ((schools!= null) &&(!schools.isEmpty())){ | ||
School school = schools.get(0); | ||
SchoolDetail schoolDetail = this.getSchoolDetailByIdFromInstituteApi(school.getSchoolId()); | ||
if ((schoolDetail != null) ) { | ||
loadSchoolDetailsIntoRedisCache(List.of(schoolDetail)); | ||
return schoolDetail ; | ||
} | ||
} | ||
} | ||
return schoolDetailTransformer.transformToDTO(schoolDetailEntity ); | ||
} | ||
|
||
public void initializeSchoolDetailCache(boolean force) { | ||
|
@@ -129,8 +203,23 @@ public void initializeSchoolDetailCache(boolean force) { | |
|
||
public List<SchoolDetail> getSchoolDetailsBySchoolCategoryCode(String schoolCategoryCode) { | ||
|
||
return schoolDetailTransformer.transformToDTO( | ||
schoolDetailRedisRepository.findBySchoolCategoryCode(schoolCategoryCode)); | ||
log.debug("**** Getting school Details By school category from Redis Cache."); | ||
List<SchoolDetailEntity> schoolDetailEntities = schoolDetailRedisRepository.findBySchoolCategoryCode(schoolCategoryCode); | ||
|
||
if ( (schoolDetailEntities == null) ||(schoolDetailEntities.isEmpty())){ | ||
log.debug("Get Schools details by category code from Redis Cache returned empty"); | ||
|
||
List<School> schools = this.getSchoolsBySearchCriteriaFromInstituteApi("schoolCategoryCode",schoolCategoryCode); | ||
ArrayList<SchoolDetail> schoolDetails = new ArrayList<>(); | ||
if ((schools != null) && (!schools.isEmpty())) { | ||
schools.forEach(school -> { | ||
schoolDetails.add(this.getSchoolDetailByIdFromInstituteApi(school.getSchoolId())); | ||
}); | ||
loadSchoolDetailsIntoRedisCache(schoolDetails); | ||
return schoolDetails; | ||
} | ||
} | ||
return schoolDetailTransformer.transformToDTO(schoolDetailEntities); | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nested catch block is unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, created a new PR #395