Skip to content

이승원 seminar0 과제#34

Open
andhl204 wants to merge 10 commits into
wafflestudio:masterfrom
andhl204:week0
Open

이승원 seminar0 과제#34
andhl204 wants to merge 10 commits into
wafflestudio:masterfrom
andhl204:week0

Conversation

@andhl204
Copy link
Copy Markdown

청강입니다!

Copy link
Copy Markdown
Member

@davin111 davin111 left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread .gitignore Outdated
.gradle/
build/

.DS_Store No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

git 마지막 newline 을 넣어주는 것이 좋은 convention 입니다.

binderFactory: WebDataBinderFactory?,
): User {
TODO()
val accessToken = webRequest.getHeader("Authorization")?.removePrefix("Bearer ") ?: throw AuthenticateException()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

HttpHeaders.AUTHORIZATION을 사용하면 하드코딩을 줄일 수 있습니다.

} catch (ex: Exception) {
when(ex) {
is SignUpBadUsernameException, is SignUpBadPasswordException -> {
ResponseEntity.status(400).build()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

status() 는 Int 대신 HttpStatus 타입도 받는데요. HttpStatus.BAD_REQUEST 를 이용하면 하드코딩을 줄일 수 있습니다.

TODO()
return try {
userService.signUp(request.username, request.password, request.image)
ResponseEntity.status(200).build()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

status(200) 대신 ok()를 쓸 수 있습니다.

Comment on lines +64 to +69
when(ex) {
is AuthenticateException -> {
ResponseEntity.status(401).build()
}
else -> throw ex
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

요기선 when 보다 단순 if 가 더 나으려나 싶기도 하네요

@Service
class UserServiceImpl(
private val userRepository: UserRepository,
private var id: Long = 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

DB 를 안 쓰시고 이렇게 id 로 구현하신 점이 흥미로운데요.

어쩌면 설정에 DB 세팅이 없어서, DB 가 없으니 자체 구현해야겠다고 생각하신 것일 수 있겠으나, Spring Boot 기본 설정에 의해 DB 를 명시하지 않았으면 인메모리 DB 인 H2 가 사용되게 됩니다.

이렇게 개발하셨어도 테스트는 성공하지만, 사실 요청이 동시에 빠르게 많이 들어오면 분명 동시성 이슈가 생깁니다. (조만간의 세미나 회차에서 더 다뤄질 거 같네요.) DB 를 안 쓰고 이런 식으로 구현하려면, 최소한 AtomicLong 을 사용하여야 합니다.

서버에서 변경이 가능한 프로퍼티인 var을 쓸 때는 항상 주의해야 합니다.

} else if (userRepository.findByUsername(username) != null) {
throw SignUpUsernameConflictException()
} else {
val user = UserEntity(id++, username, password, image)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

여기서 ++ 을 하는 것이 동시성 이슈를 만드는 부분입니다.

DB 를 안 써서 이렇게 구현하더라도 AtomicLong 의 incrementAndGet()을 사용하거나 해야합니다.

@PFCJeong PFCJeong force-pushed the master branch 2 times, most recently from 1a8e813 to f420cff Compare October 3, 2023 16:41
@PFCJeong PFCJeong force-pushed the master branch 3 times, most recently from 633df04 to 66e14db Compare October 22, 2023 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants