Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| validateDuplicate(email, nickname); | ||
| validateLength(email, VariableConfig.EMAIL_MIN, VariableConfig.EMAIL_MAX, ErrorCode.EMAIL_LENGTH_INVALID); | ||
| validateLength(nickname, VariableConfig.NICKNAME_MIN, VariableConfig.NICKNAME_MAX, ErrorCode.NICKNAME_LENGTH_INVALID); | ||
| validateLength(password, VariableConfig.PASSWORD_MIN, VariableConfig.PASSWORD_MAX, ErrorCode.PASSWORD_LENGTH_INVALID); |
There was a problem hiding this comment.
문제: findByColumn 호출 시 문자열 상수가 사용되고 있습니다. 클래스 최상단에 이미 EMAIL, NICKNAME 상수가 정의되어 있는데, 여기서는 직접 문자열을 사용하고 있습니다.
제안: 이미 정의된 상수를 활용하도록 수정하세요:
if (!userRepository.findByColumn(EMAIL, email).isEmpty())| validate(email, nickname, password); | ||
|
|
||
| User saved = userRepository.save( | ||
| new User(password, nickname, email, UserRole.MEMBER.toString())); | ||
|
|
||
| log.info("Registered id:{}, email:{}, nickname:{}, password:{}", | ||
| saved.getId(), email, nickname, password); |
There was a problem hiding this comment.
문제: 로그 메시지에 민감한 정보(비밀번호)가 포함되어 있습니다. 본 PR에서는 로그 출력을 추가했는데, 비밀번호를 그대로 출력하는 것은 보안 위험입니다.
제안: 비밀번호를 로그에 기록하지 않으세요:
log.info("Registered id:{}, email:{}, nickname:{}", saved.getId(), email, nickname);| if (Modifier.isStatic(field.getModifiers())) { | ||
| continue; | ||
| } | ||
| if ("id".equals(field.getName())) { | ||
| if ("id".equals(toColumnName(field.getName()))) { | ||
| continue; | ||
| } | ||
| sqlBuilder.append(field.getName()).append(", "); | ||
| sqlBuilder.append(toColumnName(field.getName())).append(", "); |
There was a problem hiding this comment.
문제: save() 메서드에서 "id" 필드를 필터링할 때 field.getName()과 toColumnName(field.getName())이 일관되지 않게 비교되고 있습니다.
맥락: toColumnName()은 camelCase를 snake_case로 변환하는데, "id"는 변환되지 않으므로 현재 작동하지만, 향후 혼동을 방지하기 위해 다음과 같이 수정하세요:
if ("id".equals(field.getName())) {
continue;
}camelCase 필드명으로 비교하는 게 논리적으로 더 명확합니다.
| if (Modifier.isStatic(field.getModifiers())) { | ||
| continue; | ||
| } | ||
| if ("id".equals(field.getName())) { | ||
| if ("id".equals(toColumnName(field.getName()))) { | ||
| continue; | ||
| } | ||
| sql.append(field.getName()).append(" = ?, "); | ||
| sql.append(toColumnName(field.getName())).append(" = ?, "); |
There was a problem hiding this comment.
문제: update() 메서드에서도 동일한 불일치가 있습니다. toColumnName(field.getName())으로 비교하는 것은 일관성이 없습니다.
제안: 원본 필드명으로 비교하세요:
if ("id".equals(field.getName())) {
continue;
}| } | ||
|
|
||
| public List<T> findByColumn(String columnName, Object value) { | ||
| String sql = "SELECT * FROM " + tableName + " WHERE " + columnName + " = ?"; | ||
| String sql = "SELECT * FROM " + tableName + " WHERE " + toColumnName(columnName) + " = ?"; | ||
|
|
There was a problem hiding this comment.
문제: findByColumn()에서 컬럼명 변환 시 인자값 columnName은 이미 camelCase이므로 toColumnName()을 호출해야 하는데, 이 메서드가 호출자(RegisterWithPost)에서 제대로 처리되고 있는지 확인이 필요합니다.
확인 사항: RegisterWithPost의 validateDuplicate() 메서드에서 EMAIL, NICKNAME 상수(camelCase)를 그대로 전달하고 있으므로 여기서 변환이 필요합니다. 현재 구현은 정확하지만, 호출 시 일관성을 위해 명확한 문서화가 필요합니다.
| public static final int NICKNAME_MIN = 4; | ||
| public static final int PASSWORD_MAX = 16; | ||
| public static final int PASSWORD_MIN = 4; |
There was a problem hiding this comment.
제안: 빈 줄 처리. 가독성 개선을 위해 상수 그룹 사이의 빈 줄을 하나로 줄이세요:
public static final int EMAIL_MAX = 50;
public static final int EMAIL_MIN = 4;1 participant
💻 작업 내용
🎯 관련 이슈
closed #65