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

[feat] 공통 에러 처리 및 API 응답 관련 추가 #56

Merged
Merged
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.kernel360.commoncode.code;

import com.kernel360.code.BusinessCode;
import org.springframework.http.HttpStatus;

public enum CommonCodeBusinessCode implements BusinessCode {
GET_COMMON_CODE_SUCCESS(HttpStatus.OK.value(), "BC001", "공통코드 조회 성공");

private final int status;
private final String code;
private final String message;

CommonCodeBusinessCode(int status, String code, String message) {
this.status = status;
this.code = code;
this.message = message;
}

@Override
public int getStatus() {
return status;
}

@Override
public String getCode() {
return code;
}

@Override
public String getMessage() {
return message;
}
}
Copy link

Choose a reason for hiding this comment

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

The provided code appears to be an enum class named CommonCodeBusinessCode that implements the interface BusinessCode. It defines a single enum constant GET_COMMON_CODE_SUCCESS, which has three properties: status, code, and message.

Overall, the code looks fine, but there are a few suggestions for improvement:

  1. Consider following Java naming conventions. Class names and enum constants should typically use camel case with the first letter in lowercase (e.g., CommonCodeBusinessCode can be renamed to CommonCodeBusinessCode).

  2. It would be helpful to include comments or documentation explaining the purpose of the enum and its constants.

  3. If this code is part of a larger system, it's important to ensure that error codes (code property) are unique and well-defined across the entire system.

  4. Ensure that the status values used in the enum (status property) align with the intended HTTP status codes, as 200 usually represents a successful response.

  5. Depending on your specific requirements, you might want to consider adding additional enum constants to cover different scenarios or error cases.

Remember that a thorough code review involves examining other parts of the codebase beyond just these few lines.

Copy link

Choose a reason for hiding this comment

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

The code you provided looks fine and does not seem to have any bugs. It defines an enumeration called CommonCodeBusinessCode that implements the BusinessCode interface. It has a single constant GET_COMMON_CODE_SUCCESS that represents a successful common code retrieval with a status of 200, a code of "BC001", and a message of "공통코드 조회 성공" (which translates to "Successful common code retrieval" in Korean).

Since the code snippet is small and straightforward, there are no specific improvements or bug risks to address. However, it's important to ensure that the dependencies (com.kernel360.code.BusinessCode and org.springframework.http.HttpStatus) are properly imported and available in your project.

If you have further questions or if you provide additional code snippets, I'll be happy to assist you!

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.kernel360.commoncode.code;

import com.kernel360.code.ErrorCode;
import org.springframework.http.HttpStatus;

public enum CommonCodeErrorCode implements ErrorCode {
INVALID_COMMON_CODE_NAME(HttpStatus.BAD_REQUEST.value(), "EC001", "존재하지 않는 공통코드");

private final int status;
private final String code;
private final String message;

CommonCodeErrorCode(int status, String code, String message) {
this.status = status;
this.code = code;
this.message = message;
}

@Override
public int getStatus() {
return status;
}

@Override
public String getCode() {
return code;
}

@Override
public String getMessage() {
return message;
}
}
Copy link

Choose a reason for hiding this comment

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

The code patch you provided appears to be defining an enum called CommonCodeErrorCode. It implements the ErrorCode interface and provides an error code for an invalid common code name.

Upon a brief review, I don't see any bugs or immediate issues with this code. Here are a few points to consider for improvement:

  1. Make sure the package (com.kernel360.commoncode.code) matches your project's package structure.
  2. Consider providing a more descriptive message for the error code, such as "Invalid common code name: {code}" instead of just "존재하지 않는 공통코드".
  3. Ensure that the ErrorCode interface is correctly defined and that it aligns with the usage in other parts of your codebase.

Apart from these general considerations, the code you provided seems fine. Remember to thoroughly test and integrate it into your project to confirm its correctness and adequacy for your specific needs.

9 changes: 7 additions & 2 deletions module-common/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,24 @@ repositories {
}

dependencies {
implementation 'org.springframework.boot:spring-boot-starter'
implementation 'org.projectlombok:lombok:1.18.26'
// spring-boot-starter
implementation 'org.springframework.boot:spring-boot-starter-web'
testImplementation 'org.springframework.boot:spring-boot-starter-test'

// jwt
implementation group: 'io.jsonwebtoken', name: 'jjwt-api', version: '0.11.5'
runtimeOnly group: 'io.jsonwebtoken', name: 'jjwt-impl', version: '0.11.5'
runtimeOnly group: 'io.jsonwebtoken', name: 'jjwt-jackson', version: '0.11.5'

// jasypt
implementation 'com.github.ulisesbocchio:jasypt-spring-boot-starter:3.0.5'

// lombok
implementation 'org.projectlombok:lombok:1.18.26'
annotationProcessor 'org.projectlombok:lombok'
compileOnly 'org.projectlombok:lombok'

// codec
implementation 'commons-codec:commons-codec:1.15'
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.kernel360.code;

public interface BaseCode {
int getStatus();
String getCode();
String getMessage();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.kernel360.code;

public interface BusinessCode extends BaseCode {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.kernel360.code;

public interface ErrorCode extends BaseCode {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.kernel360.code.common;

import com.kernel360.code.ErrorCode;
import org.springframework.http.HttpStatus;

public enum CommonBusinessCode implements ErrorCode {
Copy link
Collaborator

Choose a reason for hiding this comment

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

어제 이야기를 복기하다가 어디서부터 어디까지가 비즈니스 코드인지 잘 모르겠다는 생각이 들었습니다. 단순 CRUD는 비즈니스 코드로 치지 않는 것처럼 이야기하시는 분들도 있고...
저희는 Service 빈 안에 있는 코드를 모두 비즈니스 코드라고 생각하면 될까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

일단 제가 생각한 기준은 ErrorCode을 구현한 코드(Enum)들은 Exception을 던질 때 사용하게 되고, 그 외 코드들은 BusinessCode를 구현해서 업무 로직에서 로그를 남길 때 사용하는 거였어요~!
커스텀 메시지를 관리하고자 할 때, BusinessCode가 필요하다고 생각해서 에러 처리 로직을 만들면서 같이 추가하게 되었습니다!
이름에서 오는 모호함이 있다면 클래스명을 수정하는 것도 고려할 수 있다고 생각해요

SAVED(HttpStatus.OK.value(), "B001", "저장이 왼료되었습니다.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

회의 통해서 내용 확인했으며 business 쪽 코드는 관련하여서 이용하다가
추후 실제 로그만 보고 문제 파악이 힘들다던지 하는 이유 발생한다면
디테일하게 작성하는 방향으로 가는 것이 좋겠습니다. (회의에서 이야기 했지만, 까먹지 않기 위한 코맨트 작성)


private final int status;
private final String code;
private final String message;

CommonBusinessCode(int status, String code, String message) {
this.status = status;
this.code = code;
this.message = message;
}

@Override
public int getStatus() {
return status;
}

@Override
public String getCode() {
return code;
}

@Override
public String getMessage() {
return message;
}
}
Copy link

Choose a reason for hiding this comment

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

The code patch you provided appears to define an enum called CommonBusinessCode that implements the ErrorCode interface. It defines a single enum value SAVED, which represents a successful save operation with an HTTP status of 200 (OK), a code of "B001," and a message of "저장이 완료되었습니다." (which translates to "Save completed successfully").

Here are a few suggestions for improvement:

  1. Consider naming conventions: It is common practice in Java to use CamelCase for class and enum names, so you may want to consider renaming CommonBusinessCode to CommonBusinessErrorCode or something similar to conform with established conventions.

  2. Error code convention: It's generally a good idea to establish a consistent convention for error codes, such as prefixing them with a specific string or using a specific format. For example, you could use "CB001" instead of just "B001" for the error code in your SAVED enum value.

  3. Expand error handling: Currently, the code only includes a single success case. Depending on your requirements, you may need to define additional enum values to represent different error scenarios or responses.

Overall, without knowing the context or purpose of this code, it's challenging to identify any specific bug risks. However, as long as the code is used correctly and the ErrorCode interface is implemented properly, this code snippet seems fine. Remember to consider these suggestions based on your project's requirements and coding standards.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.kernel360.code.common;

import com.kernel360.code.ErrorCode;
import org.springframework.http.HttpStatus;

public enum CommonErrorCode implements ErrorCode {
INTERNAL_SERVER_ERROR(HttpStatus.INTERNAL_SERVER_ERROR.value(), "E001", "Server Error");

private final int status;
private final String code;
private final String message;

CommonErrorCode(int status, String code, String message) {
this.status = status;
this.code = code;
this.message = message;
}

@Override
public int getStatus() {
return status;
}

@Override
public String getCode() {
return code;
}

@Override
public String getMessage() {
return message;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.kernel360.exception;

import com.kernel360.code.ErrorCode;
import lombok.Getter;

@Getter
public class BusinessException extends RuntimeException {
private final ErrorCode errorCode;

public BusinessException(ErrorCode errorCode) {
this.errorCode = errorCode;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.kernel360.handler;

import com.kernel360.code.ErrorCode;
import com.kernel360.code.common.CommonErrorCode;
import com.kernel360.exception.BusinessException;
import com.kernel360.response.ErrorResponse;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;

@Slf4j
@RestControllerAdvice
public class GlobalExceptionHandler {
@ExceptionHandler(BusinessException.class)
protected ResponseEntity<ErrorResponse> handleBusinessException(final BusinessException e) {
log.error("handleBusinessException", e);

final ErrorCode errorCode = e.getErrorCode();
final ErrorResponse response = ErrorResponse.of(errorCode);

return new ResponseEntity<>(response, HttpStatus.valueOf(errorCode.getStatus()));
}

@ExceptionHandler(Exception.class)
protected ResponseEntity<ErrorResponse> handleException(Exception e) {
log.error("handleException", e);

final ErrorResponse response = ErrorResponse.of(CommonErrorCode.INTERNAL_SERVER_ERROR);

return new ResponseEntity<>(response, HttpStatus.INTERNAL_SERVER_ERROR);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.kernel360.response;

import com.kernel360.code.BusinessCode;

public record ApiResponse<T>(
int status,
String code,
String message,
T value) {

public static <T> ApiResponse<T> of(BusinessCode code, T value) {
return new ApiResponse<>(
code.getStatus(),
code.getCode(),
code.getMessage(),
value
);
}

public static <T> ApiResponse<T> of(BusinessCode code) {
return new ApiResponse<>(
code.getStatus(),
code.getCode(),
code.getMessage(),
null
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.kernel360.response;

import com.kernel360.code.ErrorCode;

public record ErrorResponse(
int status,
String code,
String message) {

public static ErrorResponse of(ErrorCode code) {
return new ErrorResponse(
code.getStatus(),
code.getCode(),
code.getMessage()
);
}
}
Loading