GRAD2-2853: code review is applied.

bcgov · Jul 18, 2024 · 4b89386 · 4b89386
1 parent f6a92d4
commit 4b89386
Show file tree

Hide file tree

Showing 28 changed files with 163 additions and 251 deletions.
diff --git a/api/src/main/java/ca/bc/gov/educ/api/course/config/RestWebClient.java b/api/src/main/java/ca/bc/gov/educ/api/course/config/RestWebClient.java
@@ -1,15 +1,13 @@
 package ca.bc.gov.educ.api.course.config;
 
 import ca.bc.gov.educ.api.course.util.EducCourseApiConstants;
-import ca.bc.gov.educ.api.course.util.LogHelper;
 import io.netty.handler.logging.LogLevel;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.security.oauth2.client.*;
 import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
 import org.springframework.security.oauth2.client.web.reactive.function.client.ServletOAuth2AuthorizedClientExchangeFilterFunction;
-import org.springframework.web.reactive.function.client.ExchangeFilterFunction;
 import org.springframework.web.reactive.function.client.ExchangeStrategies;
 import org.springframework.web.reactive.function.client.WebClient;
 import org.springframework.web.util.DefaultUriBuilderFactory;
@@ -32,11 +30,14 @@ public RestWebClient() {
         this.httpClient.warmup().block();
     }
 
-    @Bean("courseApiClient")
-    public WebClient getCourseApiClientWebClient(OAuth2AuthorizedClientManager authorizedClientManager) {
+    @Bean
+    public WebClient webClient(OAuth2AuthorizedClientManager authorizedClientManager) {
         ServletOAuth2AuthorizedClientExchangeFilterFunction filter = new ServletOAuth2AuthorizedClientExchangeFilterFunction(authorizedClientManager);
         filter.setDefaultClientRegistrationId("course-api-client");
+        DefaultUriBuilderFactory defaultUriBuilderFactory = new DefaultUriBuilderFactory();
+        defaultUriBuilderFactory.setEncodingMode(DefaultUriBuilderFactory.EncodingMode.NONE);
         return WebClient.builder()
+                .uriBuilderFactory(defaultUriBuilderFactory)
                 .exchangeStrategies(ExchangeStrategies
                         .builder()
                         .codecs(codecs -> codecs
@@ -62,28 +63,4 @@ public OAuth2AuthorizedClientManager authorizedClientManager(
         return authorizedClientManager;
     }
 
-    @Bean
-    public WebClient webClient() {
-        DefaultUriBuilderFactory defaultUriBuilderFactory = new DefaultUriBuilderFactory();
-        defaultUriBuilderFactory.setEncodingMode(DefaultUriBuilderFactory.EncodingMode.NONE);
-        return WebClient.builder().uriBuilderFactory(defaultUriBuilderFactory).exchangeStrategies(ExchangeStrategies.builder()
-                .codecs(configurer -> configurer
-                        .defaultCodecs()
-                        .maxInMemorySize(20 * 1024 * 1024)) // 20 MB
-                      .build())
-                .filter(this.log())
-                .build();
-    }
-
-    private ExchangeFilterFunction log() {
-        return (clientRequest, next) -> next
-            .exchange(clientRequest)
-            .doOnNext((clientResponse -> LogHelper.logClientHttpReqResponseDetails(
-                    clientRequest.method(),
-                    clientRequest.url().toString(),
-                    clientResponse.rawStatusCode(),
-                    clientRequest.headers().get(EducCourseApiConstants.CORRELATION_ID),
-                    constants.isSplunkLogHelperEnabled())
-            ));
-    }
 }
diff --git a/api/src/main/java/ca/bc/gov/educ/api/course/controller/CourseController.java b/api/src/main/java/ca/bc/gov/educ/api/course/controller/CourseController.java
@@ -121,10 +121,9 @@ public ResponseEntity<Course> getCourseDetailsByCourse(@PathVariable String cour
     @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "OK")})
     public ResponseEntity<List<AllCourseRequirements>> getAllCoursesRequirement(
     		@RequestParam(value = "pageNo", required = false,defaultValue = "0") Integer pageNo, 
-            @RequestParam(value = "pageSize", required = false,defaultValue = "50") Integer pageSize,
-            @RequestHeader(name="Authorization") String accessToken) {
+            @RequestParam(value = "pageSize", required = false,defaultValue = "50") Integer pageSize) {
     	logger.debug("getAllCoursesRequirement : ");
-        return response.GET(courseRequirementService.getAllCourseRequirementList(pageNo,pageSize,accessToken.replaceAll("Bearer ", "")));
+        return response.GET(courseRequirementService.getAllCourseRequirementList(pageNo,pageSize));
     }
 
     @GetMapping(EducCourseApiConstants.GET_COURSE_REQUIREMENT_BY_RULE_MAPPING)
@@ -170,10 +169,9 @@ public ResponseEntity<CourseRequirements> getCourseRequirements(
     public ResponseEntity<List<AllCourseRequirements>> getCoursesRequirementSearch(
             @RequestParam(value = "courseCode", required = false) String courseCode,
             @RequestParam(value = "courseLevel", required = false) String courseLevel,
-            @RequestParam(value = "rule", required = false) String rule,
-            @RequestHeader(name="Authorization") String accessToken) {
+            @RequestParam(value = "rule", required = false) String rule) {
     	logger.debug("getCoursesRequirementSearch : ");
-        return response.GET(courseRequirementService.getCourseRequirementSearchList(courseCode,courseLevel,rule,accessToken.replaceAll("Bearer ", "")));
+        return response.GET(courseRequirementService.getCourseRequirementSearchList(courseCode,courseLevel,rule));
     }
 
     @PostMapping(EducCourseApiConstants.GET_COURSE_REQUIREMENT_BY_COURSE_LIST_MAPPING)

diff --git a/api/src/main/java/ca/bc/gov/educ/api/course/controller/v2/CourseController.java b/api/src/main/java/ca/bc/gov/educ/api/course/controller/v2/CourseController.java
@@ -46,19 +46,19 @@ public CourseController(@Qualifier("CourseServiceV2") CourseService courseServic
     @Operation(summary = "Find a Course by Course ID",
             description = "Get a Course by Course ID", tags = { "Courses" })
     @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "OK")})
-    public ResponseEntity<Course> getCourseDetails(@PathVariable String courseID, @RequestHeader(name="Authorization") String accessToken) {
+    public ResponseEntity<Course> getCourseDetails(@PathVariable String courseID) {
         log.info("#getCourseDetails : courseID={}", courseID);
-        return response.GET(courseService.getCourseInfo(courseID, accessToken.replace(BEARER, "")));
+        return response.GET(courseService.getCourseInfo(courseID));
     }
 
     @GetMapping(EducCourseApiConstants.GET_COURSE_BY_CODE_MAPPING)
     @PreAuthorize(PermissionsConstants.READ_GRAD_COURSE)
     @Operation(summary = "Find a Course by Course Code and Course Level",
             description = "Get a Course by Course Code and Course Level", tags = { "Courses" })
     @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "OK")})
-    public ResponseEntity<Course> getCourseDetails(@PathVariable String courseCode, @PathVariable String courseLevel, @RequestHeader(name="Authorization") String accessToken) {
+    public ResponseEntity<Course> getCourseDetails(@PathVariable String courseCode, @PathVariable String courseLevel) {
         log.info("#getCourseDetails : courseCode={}, courseLevel={}", courseCode, courseLevel);
-        return response.GET(courseService.getCourseInfo(courseCode, courseLevel, accessToken.replace(BEARER, "")));
+        return response.GET(courseService.getCourseInfo(courseCode, courseLevel));
     }
 
 }
diff --git a/api/src/main/java/ca/bc/gov/educ/api/course/controller/v2/StudentCourseController.java b/api/src/main/java/ca/bc/gov/educ/api/course/controller/v2/StudentCourseController.java
@@ -49,14 +49,14 @@ public StudentCourseController(StudentCourseService studentCourseService, GradVa
     @Operation(summary = "Find All Student Courses by Student ID", description = "Get All Student Courses by Student ID", tags = {"Student Courses"})
     @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "OK"), @ApiResponse(responseCode = "204", description = "NO CONTENT")})
     public ResponseEntity<List<StudentCourse>> getStudentCoursesByStudentID(
-            @PathVariable UUID studentID, @RequestParam(value = "sortForUI", required = false, defaultValue = "false") boolean sortForUI, @RequestHeader(name="Authorization") String accessToken) {
+            @PathVariable UUID studentID, @RequestParam(value = "sortForUI", required = false, defaultValue = "false") boolean sortForUI) {
         validation.requiredField(studentID, "Student ID");
         if (validation.hasErrors()) {
             validation.stopOnErrors();
             return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
         }
         log.debug("#Get All Student Course by Student ID: {}", studentID);
-        List<StudentCourse> studentCourseList = studentCourseService.getStudentCourses(studentID, sortForUI, accessToken.replace(BEARER, ""));
+        List<StudentCourse> studentCourseList = studentCourseService.getStudentCourses(studentID, sortForUI);
         if (studentCourseList.isEmpty()) {
             return response.NO_CONTENT();
         }
@@ -72,15 +72,15 @@ public ResponseEntity<List<StudentCourse>> getStudentCoursesByStudentID(
             @ApiResponse(responseCode = "422", description = "VALIDATION ERROR")
     })
     public ResponseEntity<ApiResponseModel<StudentCourse>> saveStudentCourse(
-            @NotNull @Valid @RequestBody StudentCourse studentCourse, @RequestHeader(name="Authorization") String accessToken) {
+            @NotNull @Valid @RequestBody StudentCourse studentCourse) {
         validation.requiredField(studentCourse.getStudentID(), "Student ID");
         validation.requiredField(studentCourse.getCourseID(), "Course ID");
         if (validation.hasErrors()) {
             validation.stopOnErrors();
             return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
         }
 
-        StudentCourse result = studentCourseService.saveStudentCourse(studentCourse, accessToken.replace(BEARER, ""));
+        StudentCourse result = studentCourseService.saveStudentCourse(studentCourse);
         return response.UPDATED(result, StudentCourse.class);
     }
 

diff --git a/api/src/main/java/ca/bc/gov/educ/api/course/service/CourseRequirementService.java b/api/src/main/java/ca/bc/gov/educ/api/course/service/CourseRequirementService.java
@@ -70,7 +70,7 @@ public class CourseRequirementService {
      */
 
      @Retry(name = "generalgetcall")
-     public List<AllCourseRequirements> getAllCourseRequirementList(Integer pageNo, Integer pageSize,String accessToken) {
+     public List<AllCourseRequirements> getAllCourseRequirementList(Integer pageNo, Integer pageSize) {
         List<CourseRequirement> courseReqList  = new ArrayList<>();
         List<AllCourseRequirements> allCourseRequiremntList = new ArrayList<>();
         try {  
@@ -85,7 +85,7 @@ public List<AllCourseRequirements> getAllCourseRequirementList(Integer pageNo, I
         		if(course != null) {
         			obj.setCourseName(course.getCourseName());
         		}
-                List<GradRuleDetails> ruleList = getRuleDetails(cR.getRuleCode().getCourseRequirementCode(), accessToken);
+                List<GradRuleDetails> ruleList = getRuleDetails(cR.getRuleCode().getCourseRequirementCode());
             	StringBuilder requirementProgram = getRequirementProgram(ruleList,obj);
 
             	obj.setRequirementProgram(requirementProgram.toString());
@@ -182,7 +182,7 @@ public CourseRequirements getCourseRequirementListByCourses(CourseList courseLis
 	}
 
     @Retry(name = "searchcoursecall")
-	public List<AllCourseRequirements> getCourseRequirementSearchList(String courseCode, String courseLevel, String rule,String accessToken) {
+	public List<AllCourseRequirements> getCourseRequirementSearchList(String courseCode, String courseLevel, String rule) {
 		CriteriaHelper criteria = new CriteriaHelper();
         getSearchCriteria("courseCode", courseCode, criteria);
         getSearchCriteria("courseLevel", courseLevel, criteria);
@@ -200,7 +200,7 @@ public List<AllCourseRequirements> getCourseRequirementSearchList(String courseC
         		if(course != null) {
         			obj.setCourseName(course.getCourseName());
         		}
-                List<GradRuleDetails> ruleList = getRuleDetails(cR.getRuleCode().getCourseRequirementCode(), accessToken);
+                List<GradRuleDetails> ruleList = getRuleDetails(cR.getRuleCode().getCourseRequirementCode());
             	StringBuilder requirementProgram = getRequirementProgram(ruleList,obj);
             	obj.setTraxReqNumber(!ruleList.isEmpty()?ruleList.get(0).getTraxReqNumber():null);
             	obj.setRequirementProgram(requirementProgram.toString());
@@ -274,9 +274,8 @@ public CourseRequirement saveCourseRequirement(CourseRequirement courseRequireme
         }
     }
 
-    private List<GradRuleDetails> getRuleDetails(String ruleCode, String accessToken) {
-        List<Map> response = restService.get(String.format(constants.getRuleDetailProgramManagementApiUrl(), ruleCode),
-                List.class, accessToken);
+    private List<GradRuleDetails> getRuleDetails(String ruleCode) {
+        List<Map> response = restService.get(String.format(constants.getRuleDetailProgramManagementApiUrl(), ruleCode), List.class);
         return jsonTransformer.convertValue(response, new TypeReference<>(){});
     }
 

diff --git a/api/src/main/java/ca/bc/gov/educ/api/course/service/RESTService.java b/api/src/main/java/ca/bc/gov/educ/api/course/service/RESTService.java
@@ -4,7 +4,6 @@
 import ca.bc.gov.educ.api.course.util.EducCourseApiConstants;
 import ca.bc.gov.educ.api.course.util.ThreadLocalStateUtil;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.HttpStatusCode;
 import org.springframework.stereotype.Service;
@@ -19,62 +18,14 @@
 @Service
 public class RESTService {
     private final WebClient webClient;
-    private final WebClient courseApiWebClient;
     private static final String ERROR_5xx = "5xx error.";
     private static final String SERVICE_FAILED_ERROR = "Service failed to process after max retries.";
 
     @Autowired
-    public RESTService(@Qualifier("courseApiClient") WebClient courseApiWebClient, WebClient webClient) {
+    public RESTService(WebClient webClient) {
         this.webClient = webClient;
-        this.courseApiWebClient = courseApiWebClient;
     }
 
-    /**
-     * Generic GET call out to services. Uses blocking webclient and will throw
-     * runtime exceptions. Will attempt retries if 5xx errors are encountered.
-     * You can catch Exception in calling method.
-     *
-     * NOTE: Soon to be deprecated in favour of calling get method without access token below.
-     *
-     * @param url the url you are calling
-     * @param clazz the return type you are expecting
-     * @param accessToken access token
-     * @return return type
-     * @param <T> expected return type
-     * @deprecated use the one without accessToken instead
-     */
-    public <T> T get(String url, Class<T> clazz, String accessToken) {
-        T obj;
-        try {
-            obj = webClient
-                    .get()
-                    .uri(url)
-                    .headers(h -> { h.setBearerAuth(accessToken); h.set(EducCourseApiConstants.CORRELATION_ID, ThreadLocalStateUtil.getCorrelationID()); })
-                    .retrieve()
-                    // if 5xx errors, throw Service error
-                    .onStatus(HttpStatusCode::is5xxServerError,
-                            clientResponse -> Mono.error(new ServiceException(getErrorMessage(url, ERROR_5xx), 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;
-    }
-
-
     /**
      * Uses this method with a service client
      * @param url
@@ -85,7 +36,7 @@ public <T> T get(String url, Class<T> clazz, String accessToken) {
     public <T> T get(String url, Class<T> clazz) {
         T obj;
         try {
-            obj = courseApiWebClient
+            obj = webClient
                     .get()
                     .uri(url)
                     .headers(h -> { h.set(EducCourseApiConstants.CORRELATION_ID, ThreadLocalStateUtil.getCorrelationID()); })
@@ -113,21 +64,12 @@ public <T> T get(String url, Class<T> clazz) {
         return obj;
     }
 
-    /**
-     * NOTE: Soon to be deprecated in favour of calling get method without access token below.
-     * @param url
-     * @param body
-     * @param clazz
-     * @param accessToken
-     * @return
-     * @param <T>
-     */
-    public <T> T post(String url, Object body, Class<T> clazz, String accessToken) {
+    public <T> T post(String url, Object body, Class<T> clazz) {
         T obj;
         try {
             obj = webClient.post()
                     .uri(url)
-                    .headers(h -> { h.setBearerAuth(accessToken); h.set(EducCourseApiConstants.CORRELATION_ID, ThreadLocalStateUtil.getCorrelationID()); })
+                    .headers(h -> { h.set(EducCourseApiConstants.CORRELATION_ID, ThreadLocalStateUtil.getCorrelationID()); })
                     .body(BodyInserters.fromValue(body))
                     .retrieve()
                     .onStatus(HttpStatusCode::is5xxServerError,
@@ -149,7 +91,6 @@ public <T> T post(String url, Object body, Class<T> clazz, String accessToken) {
         return obj;
     }
 
-
     private String getErrorMessage(String url, String errorMessage) {
         return "Service failed to process at url: " + url + " due to: " + errorMessage;
     }