Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions api/src/main/java/ca/bc/gov/educ/api/trax/service/RESTService.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@
import org.springframework.web.reactive.function.client.WebClient;
import org.springframework.web.reactive.function.client.WebClientResponseException;
import reactor.core.publisher.Mono;
import org.springframework.web.util.UriComponentsBuilder;
import reactor.util.retry.Retry;

import java.time.Duration;
import java.util.HashMap;
import java.util.List;

@Service
public class RESTService {
Expand Down Expand Up @@ -102,6 +105,42 @@ public <T> T get(String url, Class<T> clazz, WebClient webClient) {
return obj;
}

public <T> T get(String url, HashMap<String, String >params, Class<T> clazz, WebClient webClient) {
T obj;
if (webClient == null)
webClient = this.webClient;
try {
UriComponentsBuilder uriBuilder = UriComponentsBuilder.fromUriString(url);
params.forEach((key, value) -> uriBuilder.queryParam(key, value));
String uri = uriBuilder.build().toUriString();

obj = webClient
.get()
.uri(uri)
.headers(h -> { h.set(EducGradTraxApiConstants.CORRELATION_ID, ThreadLocalStateUtil.getCorrelationID()); })
.retrieve()
// if 5xx errors, throw Service error
.onStatus(HttpStatusCode::is5xxServerError,
clientResponse -> Mono.error(new ServiceException(getErrorMessage(url, SERVER_ERROR), clientResponse.statusCode().value())))
.bodyToMono(clazz)
// only does retry if initial error was 5xx as service may be temporarily down
// 4xx errors will always happen if 404, 401, 403 etc, so does not retry
.retryWhen(Retry.backoff(3, Duration.ofSeconds(2))
.filter(ServiceException.class::isInstance)
.onRetryExhaustedThrow((retryBackoffSpec, retrySignal) -> {
throw new ServiceException(getErrorMessage(url, SERVICE_FAILED_ERROR), HttpStatus.SERVICE_UNAVAILABLE.value());
}))
.block();
} catch (Exception e) {
// catches IOExceptions and the like
throw new ServiceException(
getErrorMessage(url, e.getLocalizedMessage()),
(e instanceof WebClientResponseException) ? ((WebClientResponseException) e).getStatusCode().value() : HttpStatus.SERVICE_UNAVAILABLE.value(),
e);
}
return obj;
}

/**
* NOTE: Soon to be deprecated in favour of calling get method without access token below.
* @param url
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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")
Expand Down Expand Up @@ -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 {
Copy link
Contributor

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.

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

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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

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

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();
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

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

if ( (!districtEntities.iterator().hasNext())){
log.debug("Get District from Redis Cache returned empty");
List<District> districts = this.getDistrictsFromInstituteApi();
if ((districts != null) && (!districts .isEmpty())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment on getDistrictByIdFromRedisCache method regarding null checks on Optional

Choose a reason for hiding this comment

The 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) {
Expand All @@ -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())){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment on getDistrictByIdFromRedisCache method regarding null checks on Optional

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

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())) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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:

log.debug("**** Getting district by ID from Redis Cache.");
return districtRedisRepository.findById(districtId)
        .map(districtTransformer::transformToDTO)
        .orElseGet(() -> {
            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;
        });

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

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;

Expand All @@ -93,6 +155,20 @@ public List<District> getDistrictsBySchoolCategoryCode(String schoolCategoryCode
return districts;
}

public District getDistrictByIdFromInstituteApi(String districtId) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Author

@JavaDeveloper456788 JavaDeveloper456788 Jan 14, 2025

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. 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

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

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())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment on DistrictService.getDistrictByIdFromRedisCache method regarding null checks on Optional

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

School school = schools.get(0);
if (school != null) {
loadSchoolsIntoRedisCache(List.of(school));
return school;
}
}
}
return schoolTransformer.transformToDTO(schoolEntity);
}

public boolean checkIfSchoolExists(String minCode) {
Expand All @@ -97,6 +121,33 @@ public SchoolDetail getSchoolDetailByIdFromInstituteApi(String schoolId) {
return null;
}



public List <School> getSchoolsBySearchCriteriaFromInstituteApi(String key, String value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments on getDistrictsBySearchCriteriaFromInstituteApi. Please change accordingly.

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

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();
Expand All @@ -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());

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

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

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) {
Expand All @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,12 @@ public class EducGradTraxApiConstants {
@Value("${endpoint.student-admin.authority-details.url}")
private String studentAdminAuthorityDetailsUrl;

@Value("${endpoint.institute-api.get-schools-paginated.url}")
private String schoolsPaginated;

@Value("${endpoint.institute-api.get-districts-paginated.url}")
private String districtsPaginated;

// Scheduler: ongoing updates from TRAX to GRAD
@Value("${cron.scheduled.process.events.trax-to-grad.run}")
private String traxToGradCronRun;
Expand Down
Loading
Loading