Implemented fallback mechanism to TRAX v2 endpoints that obtain data… #394
Conversation
…from redis cache for school and district service, added unit tests, added new endpoints
|
| 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.
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.
Fixed, created a new PR #395
| * @throws JsonProcessingException the json process exception | ||
| * | ||
| */ | ||
| public static HashMap<String, String> searchStringsToHTTPParams(HashMap<String, String> searchStringMap) throws JsonProcessingException { |
There was a problem hiding this comment.
Consume and return an interface (Map) instead of an implementation
There was a problem hiding this comment.
Fixed, created a new PR #395
| HashMap<String, String> searchInput = new HashMap<>(); | ||
| searchInput.put(key, value); | ||
|
|
||
| try { |
There was a problem hiding this comment.
Nested catch block is unnecessary.
There was a problem hiding this comment.
Fixed, created a new PR #395
| 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.
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.
Fixed, created a new PR #395
| 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.
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.
Fixed, created a new PR #395
| 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.
See comment on getDistrictByIdFromRedisCache method regarding null checks on Optional
There was a problem hiding this comment.
Fixed created a new PR #395
| @@ -93,6 +155,20 @@ public List<District> getDistrictsBySchoolCategoryCode(String schoolCategoryCode | |||
| return districts; | |||
| } | |||
|
|
|||
| public District getDistrictByIdFromInstituteApi(String districtId) { | |||
There was a problem hiding this comment.
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.
Fixed created a new PR #395. The method already returns District object
| 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.
See comment on DistrictService.getDistrictByIdFromRedisCache method regarding null checks on Optional
There was a problem hiding this comment.
Fixed created a new PR #395
| @@ -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.
See comments on getDistrictsBySearchCriteriaFromInstituteApi. Please change accordingly.
There was a problem hiding this comment.
Fixed created a new PR #395
| } | ||
|
|
||
| public List<SchoolDetail> getSchoolDetailsFromRedisCache() { | ||
| log.debug("**** Getting school Details from Redis Cache."); | ||
| return schoolDetailTransformer.transformToDTO(schoolDetailRedisRepository.findAll()); | ||
|
|
There was a problem hiding this comment.
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.
Fixed created a new PR #395
… from redis cache for school and district service, added unit tests, added new endpoints