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

GRAD2-2768 #348

Merged
merged 33 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
132e26b
Added an extra type hierarchy for school events for filtering on tran…
Aug 15, 2024
dd34f32
Added some unit tests to support history filtering on school created …
Aug 15, 2024
7b3e35e
Added testing for school updated
Aug 15, 2024
5659c42
Added testing and code for move school filtering
Aug 15, 2024
709db5f
Added rules for UPDATE_SCHOOL_CONTACT
Aug 15, 2024
96606fe
Removed old update event with history method.
Aug 15, 2024
e43521e
Merge branch 'refs/heads/grad-release' into GRAD2-2768
Aug 20, 2024
b729386
Updated gitignore
Aug 22, 2024
8f9096d
initial commit
Aug 22, 2024
47f72e6
Added endpoint for searching event history.
Aug 22, 2024
d9da5cc
Added openapi specs
Aug 22, 2024
08909da
Added a mapper
Aug 26, 2024
41819a4
Added json ignore to prevent infinite recursion
Aug 26, 2024
df10472
Changed to Event, also updated pom to include mapstruct annotation pr…
Aug 26, 2024
874003b
Still buggered mapper
Aug 28, 2024
a453013
Success with mapper
Aug 28, 2024
c677112
Updated test file and pom
Aug 28, 2024
21717e2
Updated exclusion for mapper
Aug 28, 2024
cc07256
Updated exclusion for filter
Aug 28, 2024
3e948ca
Adding unit testing for service
Aug 28, 2024
f3d3ab5
Increasing test coverage.
Aug 29, 2024
2b185fe
Merge branch 'refs/heads/grad-release' into GRAD2-2768
Aug 29, 2024
af8eddf
Increasing test coverage.
Aug 29, 2024
bcd13ef
Including for test
Aug 29, 2024
4bd315b
Including mapper tests
Aug 29, 2024
cef7a35
Added update endpoint and service method
Aug 30, 2024
40b5b15
Added tests and validation for EventHistory
Aug 30, 2024
9e561c4
Finished integration testing
Aug 30, 2024
a60408b
Excluding filter
Aug 30, 2024
b1561ec
Added validation response formatter and increased testing on UUIDMapper
Aug 30, 2024
75cba22
Added routes to student admin
Aug 30, 2024
4103ba2
Updated EntityNotFound handler
Sep 9, 2024
45c9372
Cleaning up code
Sep 9, 2024
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
6 changes: 4 additions & 2 deletions .github/workflows/build.from.dev.branch.deploy.to.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ jobs:
${{ env.COMMON_NAMESPACE }} \
${{ env.BUSINESS_NAMESPACE }} \
${{ secrets.SPLUNK_TOKEN }} \
${{ vars.APP_LOG_LEVEL }}
${{ vars.APP_LOG_LEVEL }} \
${{ secrets.STUDENT_ADMIN_URL_ROOT }}

# OVERRIDE Configmaps
curl -s https://raw.githubusercontent.com/bcgov/${{ env.REPO_NAME }}/${{ github.event.inputs.choice }}/tools/config/override-configmap-dev.sh \
Expand All @@ -139,7 +140,8 @@ jobs:
${{ env.COMMON_NAMESPACE }} \
${{ env.BUSINESS_NAMESPACE }} \
${{ secrets.SPLUNK_TOKEN }} \
${{ vars.APP_LOG_LEVEL }}
${{ vars.APP_LOG_LEVEL }} \
${{ secrets.STUDENT_ADMIN_URL_ROOT }}

# Start rollout (if necessary) and follow it
oc rollout latest dc/${{ env.SPRING_BOOT_IMAGE_NAME }} 2> /dev/null \
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/build.from.main.branch.deploy.to.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ jobs:
${{ env.COMMON_NAMESPACE }} \
${{ env.BUSINESS_NAMESPACE }} \
${{ secrets.SPLUNK_TOKEN }} \
${{ vars.APP_LOG_LEVEL }}
${{ vars.APP_LOG_LEVEL }} \
${{ secrets.STUDENT_ADMIN_URL_ROOT }}

# OVERRIDE Configmaps
curl -s https://raw.githubusercontent.com/bcgov/${{ env.REPO_NAME }}/${{ env.BRANCH }}/tools/config/override-configmap-dev.sh \
Expand All @@ -127,7 +128,8 @@ jobs:
${{ env.COMMON_NAMESPACE }} \
${{ env.BUSINESS_NAMESPACE }} \
${{ secrets.SPLUNK_TOKEN }} \
${{ vars.APP_LOG_LEVEL }}
${{ vars.APP_LOG_LEVEL }} \
${{ secrets.STUDENT_ADMIN_URL_ROOT }}

# Start rollout (if necessary) and follow it
oc rollout latest dc/${{ env.SPRING_BOOT_IMAGE_NAME }} 2> /dev/null \
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/build.from.release.branch.deploy.to.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ jobs:
${{ env.COMMON_NAMESPACE }} \
${{ env.BUSINESS_NAMESPACE }} \
${{ secrets.SPLUNK_TOKEN }} \
${{ vars.APP_LOG_LEVEL }}
${{ vars.APP_LOG_LEVEL }} \
${{ secrets.STUDENT_ADMIN_URL_ROOT }}

# OVERRIDE Configmaps
curl -s https://raw.githubusercontent.com/bcgov/${{ env.REPO_NAME }}/${{ env.BRANCH }}/tools/config/override-configmap-dev.sh \
Expand All @@ -134,7 +135,8 @@ jobs:
${{ env.COMMON_NAMESPACE }} \
${{ env.BUSINESS_NAMESPACE }} \
${{ secrets.SPLUNK_TOKEN }} \
${{ vars.APP_LOG_LEVEL }}
${{ vars.APP_LOG_LEVEL }} \
${{ secrets.STUDENT_ADMIN_URL_ROOT }}

# Start rollout (if necessary) and follow it
oc rollout latest dc/${{ env.SPRING_BOOT_IMAGE_NAME }} 2> /dev/null \
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/deploy_prod.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ jobs:
${{ env.COMMON_NAMESPACE }} \
${{ env.BUSINESS_NAMESPACE }} \
${{ secrets.SPLUNK_TOKEN }} \
${{ vars.APP_LOG_LEVEL }}
${{ vars.APP_LOG_LEVEL }} \
${{ secrets.STUDENT_ADMIN_URL_ROOT }}

# Start rollout (if necessary) and follow it
oc rollout latest dc/${{ env.SPRING_BOOT_IMAGE_NAME }} 2> /dev/null \
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/deploy_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ jobs:
${{ env.COMMON_NAMESPACE }} \
${{ env.BUSINESS_NAMESPACE }} \
${{ secrets.SPLUNK_TOKEN }} \
${{ vars.APP_LOG_LEVEL }}
${{ vars.APP_LOG_LEVEL }} \
${{ secrets.STUDENT_ADMIN_URL_ROOT }}

# Start rollout (if necessary) and follow it
oc rollout latest dc/${{ env.SPRING_BOOT_IMAGE_NAME }} 2> /dev/null \
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,5 @@ build/
### VS Code ###
.vscode/

### local dev ###
### Local dev ###
**/application-local.yaml
20 changes: 18 additions & 2 deletions api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@
src/main/java/ca/bc/gov/educ/api/trax/model/**,
src/main/java/ca/bc/gov/educ/api/trax/service/EventHandlerDelegatorService,
src/main/java/ca/bc/gov/educ/api/trax/util/**,
src/main/java/ca/bc/gov/educ/api/trax/repository/**
src/main/java/ca/bc/gov/educ/api/trax/repository/**,
src/main/java/ca/bc/gov/educ/api/trax/filter/**
</sonar.exclusions>
<sonar.junit.exclusions>
src/main/java/ca/bc/gov/educ/api/trax/messaging/**,
src/main/java/ca/bc/gov/educ/api/trax/scheduler/**,
src/main/java/ca/bc/gov/educ/api/trax/model/**,
src/main/java/ca/bc/gov/educ/api/trax/util/**,
src/main/java/ca/bc/gov/educ/api/trax/repository/**
src/main/java/ca/bc/gov/educ/api/trax/repository/**,
src/main/java/ca/bc/gov/educ/api/trax/filter/**
</sonar.junit.exclusions>
<java.version>18</java.version>
<maven.compiler.version>3.10.1</maven.compiler.version>
Expand Down Expand Up @@ -71,6 +73,11 @@
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-data-jpa</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.security</groupId>
<artifactId>spring-security-test</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-data-redis</artifactId>
Expand All @@ -83,6 +90,10 @@
<groupId>org.apache.commons</groupId>
<artifactId>commons-pool2</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-validation</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-api</artifactId>
Expand Down Expand Up @@ -322,6 +333,11 @@
<artifactId>lombok</artifactId>
<version>${lombok.version}</version>
</path>
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok-mapstruct-binding</artifactId>
<version>0.2.0</version>
</dependency>
<path>
<groupId>org.springframework</groupId>
<artifactId>spring-context-indexer</artifactId>
Expand Down
121 changes: 78 additions & 43 deletions api/src/main/java/ca/bc/gov/educ/api/trax/config/RestErrorHandler.java
Original file line number Diff line number Diff line change
@@ -1,63 +1,99 @@
package ca.bc.gov.educ.api.trax.config;

import ca.bc.gov.educ.api.trax.exception.ApiError;
import ca.bc.gov.educ.api.trax.exception.EntityNotFoundException;
import ca.bc.gov.educ.api.trax.exception.GradBusinessRuleException;
import ca.bc.gov.educ.api.trax.util.ApiResponseMessage.MessageTypeEnum;
import ca.bc.gov.educ.api.trax.util.ApiResponseModel;
import ca.bc.gov.educ.api.trax.util.GradValidation;
import lombok.extern.slf4j.Slf4j;
import org.hibernate.dialect.lock.OptimisticEntityLockException;
import org.hibernate.exception.ConstraintViolationException;
import org.jboss.logging.Logger;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.dao.DataRetrievalFailureException;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.HttpStatusCode;
import org.springframework.http.ResponseEntity;
import org.springframework.orm.jpa.JpaObjectRetrievalFailureException;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.validation.FieldError;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.context.request.WebRequest;
import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;

import java.util.HashMap;
import java.util.Map;

import static org.springframework.http.HttpStatus.BAD_REQUEST;
import static org.springframework.http.HttpStatus.NOT_FOUND;

@Slf4j
@ControllerAdvice
public class RestErrorHandler extends ResponseEntityExceptionHandler {

private static final Logger LOGGER = Logger.getLogger(RestErrorHandler.class);
GradValidation validation;

@Autowired
GradValidation validation;
public RestErrorHandler(GradValidation validation) {
this.validation = validation;
}

/**
* Override method handles exception thrown by @Valid validation
* on controller methods (for example)
*/
@Override
protected ResponseEntity<Object> handleMethodArgumentNotValid(MethodArgumentNotValidException ex, HttpHeaders headers, HttpStatusCode status, WebRequest request) {
Map<String, String> errors = new HashMap<>();
ex.getBindingResult().getAllErrors().forEach(error -> {
String fieldName = ((FieldError) error).getField();
String errorMessage = error.getDefaultMessage();
errors.put(fieldName, errorMessage);
});
ApiError apiError = new ApiError(BAD_REQUEST);
apiError.setMessage(errors.toString());
return buildResponseEntity(apiError);
}

@ExceptionHandler(value = { IllegalArgumentException.class, IllegalStateException.class })
protected ResponseEntity<Object> handleConflict(RuntimeException ex, WebRequest request) {
LOGGER.error("Illegal argument ERROR IS: " + ex.getClass().getName(), ex);
protected ResponseEntity<Object> handleConflict(RuntimeException ex) {
log.error("Illegal argument ERROR IS: {}", ex.getClass().getName(), ex);
ApiResponseModel<?> response = ApiResponseModel.ERROR(null, ex.getLocalizedMessage());
validation.ifErrors(errorList -> response.addErrorMessages(errorList));
validation.ifWarnings(warningList -> response.addWarningMessages(warningList));
validation.ifErrors(response::addErrorMessages);
validation.ifWarnings(response::addWarningMessages);
validation.clear();
return new ResponseEntity<>(response, HttpStatus.UNPROCESSABLE_ENTITY);
}

@ExceptionHandler(value = { JpaObjectRetrievalFailureException.class, DataRetrievalFailureException.class })
protected ResponseEntity<Object> handleEntityNotFound(RuntimeException ex, WebRequest request) {
LOGGER.error("JPA ERROR IS: " + ex.getClass().getName(), ex);
validation.clear();
return new ResponseEntity<>(ApiResponseModel.ERROR(null, ex.getLocalizedMessage()), HttpStatus.BAD_REQUEST);
@ExceptionHandler(value = { JpaObjectRetrievalFailureException.class, DataRetrievalFailureException.class, EntityNotFoundException.class })
protected ResponseEntity<Object> handleEntityNotFound(RuntimeException ex) {
// no need to log EntityNotFoundExceptions as error
// it is used to denote NOT FOUND and is normal part of operations
if(!(ex instanceof EntityNotFoundException)){
log.error("JPA ERROR IS: {}", ex.getClass().getName(), ex);
}
ApiError apiError = new ApiError(NOT_FOUND);
apiError.setMessage(ex.getMessage());
return buildResponseEntity(apiError);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the second EntityNotFound handler, but covers DataRetrievalFailure.
Also DataRetrievalFailure is more related to the server side 500 group of errors and returning of BAD_REQUEST is confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. I merged the two and return a 404. The EntityNotFoundException is the only one explicitly thrown to state that an object is not found in the DB, while the other exceptions are from Spring JPA but also denote not found.


@ExceptionHandler(value = { AccessDeniedException.class })
protected ResponseEntity<Object> handleAuthorizationErrors(Exception ex, WebRequest request) {

LOGGER.error("Authorization error EXCETPION IS: " + ex.getClass().getName());
protected ResponseEntity<Object> handleAuthorizationErrors(AccessDeniedException ex) {
log.error("Authorization error EXCEPTION IS: {}", ex.getClass().getName());
String message = "You are not authorized to access this resource.";
validation.clear();
return new ResponseEntity<>(ApiResponseModel.ERROR(null, message), HttpStatus.FORBIDDEN);
}

@ExceptionHandler(value = { GradBusinessRuleException.class })
protected ResponseEntity<Object> handleGradBusinessException(Exception ex, WebRequest request) {
protected ResponseEntity<Object> handleGradBusinessException(GradBusinessRuleException ex) {
ApiResponseModel<?> response = ApiResponseModel.ERROR(null);
validation.ifErrors(errorList -> response.addErrorMessages(errorList));
validation.ifWarnings(warningList -> response.addWarningMessages(warningList));
validation.ifErrors(response::addErrorMessages);
validation.ifWarnings(response::addWarningMessages);
if (response.getMessages().isEmpty()) {
response.addMessageItem(ex.getLocalizedMessage(), MessageTypeEnum.ERROR);
}
Expand All @@ -66,55 +102,54 @@ protected ResponseEntity<Object> handleGradBusinessException(Exception ex, WebRe
}

@ExceptionHandler(value = { OptimisticEntityLockException.class })
protected ResponseEntity<Object> handleOptimisticEntityLockException(OptimisticEntityLockException ex, WebRequest request) {

LOGGER.error("EXCEPTION IS: " + ex.getClass().getName(), ex);
LOGGER.error("Illegal argument ERROR IS: " + ex.getClass().getName(), ex);
ApiResponseModel<?> response = ApiResponseModel.ERROR(null);
validation.ifErrors(errorList -> response.addErrorMessages(errorList));
validation.ifWarnings(warningList -> response.addWarningMessages(warningList));
if (!validation.hasErrors()) {
response.addMessageItem(ex.getLocalizedMessage(), MessageTypeEnum.ERROR);
}
validation.clear();
return new ResponseEntity<>(response, HttpStatus.BAD_REQUEST);
protected ResponseEntity<Object> handleOptimisticEntityLockException(OptimisticEntityLockException ex) {
return getGenericUncaughtExceptionResponse(ex);
}

@ExceptionHandler(value = { DataIntegrityViolationException.class })
protected ResponseEntity<Object> handleSQLException(DataIntegrityViolationException ex, WebRequest request) {
protected ResponseEntity<Object> handleSQLException(DataIntegrityViolationException ex) {

LOGGER.error("DATA INTEGRITY VIOLATION IS: " + ex.getClass().getName(), ex);
log.error("DATA INTEGRITY VIOLATION IS: {}", ex.getClass().getName(), ex);
String msg = ex.getLocalizedMessage();

Throwable cause = ex.getCause();
if (cause instanceof ConstraintViolationException) {
ConstraintViolationException contraintViolation = (ConstraintViolationException) cause;
if ("23000".equals(contraintViolation.getSQLState())) {
// primary key violation - probably inserting a duplicate record
msg = ex.getRootCause().getMessage();
msg = (ex.getRootCause() != null) ? ex.getRootCause().getMessage() : "";
}
}

ApiResponseModel<?> response = ApiResponseModel.ERROR(null, msg);
validation.ifErrors(errorList -> response.addErrorMessages(errorList));
validation.ifWarnings(warningList -> response.addWarningMessages(warningList));
validation.ifErrors(response::addErrorMessages);
validation.ifWarnings(response::addWarningMessages);
validation.clear();
return new ResponseEntity<>(response, HttpStatus.UNPROCESSABLE_ENTITY);
}

@ExceptionHandler(value = { Exception.class })
protected ResponseEntity<Object> handleUncaughtException(Exception ex, WebRequest request) {
protected ResponseEntity<Object> handleUncaughtException(Exception ex) {
return getGenericUncaughtExceptionResponse(ex);
}

LOGGER.error("EXCEPTION IS: " + ex.getClass().getName(), ex);
LOGGER.error("Illegal argument ERROR IS: " + ex.getClass().getName(), ex);
private ResponseEntity<Object> getGenericUncaughtExceptionResponse(Exception ex) {
log.error("EXCEPTION IS: {}", ex.getClass().getName(), ex);
ApiResponseModel<?> response = ApiResponseModel.ERROR(null);
validation.ifErrors(errorList -> response.addErrorMessages(errorList));
validation.ifWarnings(warningList -> response.addWarningMessages(warningList));
validation.ifErrors(response::addErrorMessages);
validation.ifWarnings(response::addWarningMessages);
if (!validation.hasErrors()) {
response.addMessageItem(ex.getLocalizedMessage(), MessageTypeEnum.ERROR);
}
validation.clear();
return new ResponseEntity<>(response, HttpStatus.BAD_REQUEST);
}


/**
* Build response entity response entity.
*
* @param apiError the api error
* @return the response entity
*/
private ResponseEntity<Object> buildResponseEntity(ApiError apiError) {
return new ResponseEntity<>(apiError, apiError.getStatus());
}
}
Loading
Loading