-
Notifications
You must be signed in to change notification settings - Fork 0
merge:book api #12
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
merge:book api #12
Changes from all commits
2c94978
4fd0836
cd72e13
324826b
edb8c10
0441cf6
581e923
1f25cfb
f9e1792
f8e14f9
4e20b02
b1073de
9518e59
8ed3638
c7b25a4
b1b94aa
db5b336
f6823f2
5b1a221
f935895
d656f43
1ff0c5f
a8b74f0
5ef2af1
d29e3c3
5aef755
38d7a91
d2f50fa
d147351
9b58927
a913dda
762eb20
99a9f2a
21230aa
bafd9a2
e9f8da4
0a8f61a
ff2a370
a3a659b
b5fb654
6174174
d1d9c57
b04c44a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| package org.poolc.api.book.client; | ||
|
|
||
| import org.poolc.api.book.dto.response.BookApiResponse; | ||
|
|
||
| import javax.management.modelmbean.XMLParseException; | ||
| import java.util.List; | ||
|
|
||
| public interface BookClient { | ||
|
|
||
| List<BookApiResponse> searchBooks(String query, int page) throws XMLParseException; | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,64 @@ | ||||||||||||||||||||
| package org.poolc.api.book.client; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import com.fasterxml.jackson.dataformat.xml.XmlMapper; | ||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||
| import org.poolc.api.book.dto.response.BookApiResponse; | ||||||||||||||||||||
| import org.poolc.api.book.dto.response.NaverApiResponse; | ||||||||||||||||||||
| import org.springframework.beans.factory.annotation.Value; | ||||||||||||||||||||
| import org.springframework.http.HttpEntity; | ||||||||||||||||||||
| import org.springframework.http.HttpHeaders; | ||||||||||||||||||||
| import org.springframework.http.HttpMethod; | ||||||||||||||||||||
| import org.springframework.stereotype.Component; | ||||||||||||||||||||
| import org.springframework.web.client.RestTemplate; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import javax.management.modelmbean.XMLParseException; | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Use appropriate exception type for XML parsing
Options:
π€ Prompt for AI Agents |
||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Component | ||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||
| public class NaverBookClient implements BookClient{ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Value("${book.api.url}") | ||||||||||||||||||||
| private String url; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Value("${book.api.secret}") | ||||||||||||||||||||
| private String clientSecret; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Value("${book.api.id}") | ||||||||||||||||||||
| private String clientId; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private final RestTemplate restTemplate; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private static final int PAGE_SIZE = 10; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| public List<BookApiResponse> searchBooks(String query, int page) throws XMLParseException { | ||||||||||||||||||||
| HttpHeaders headers = new HttpHeaders(); | ||||||||||||||||||||
| headers.set("X-Naver-Client-Id", clientId); | ||||||||||||||||||||
| headers.set("X-Naver-Client-Secret", clientSecret); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| HttpEntity<String> entity = new HttpEntity<>(headers); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| String url = new StringBuilder(this.url) | ||||||||||||||||||||
| .append("?query=").append(query) | ||||||||||||||||||||
| .append("&display=").append(PAGE_SIZE) | ||||||||||||||||||||
| .append("&start=").append(page * PAGE_SIZE + 1) | ||||||||||||||||||||
| .toString(); | ||||||||||||||||||||
|
Comment on lines
+42
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. URL encode query parameter to handle special characters The query parameter should be URL encoded to properly handle special characters, spaces, and non-ASCII text. Add URL encoding: +import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
String url = new StringBuilder(this.url)
- .append("?query=").append(query)
+ .append("?query=").append(URLEncoder.encode(query, StandardCharsets.UTF_8))
.append("&display=").append(PAGE_SIZE)
.append("&start=").append(page * PAGE_SIZE + 1)
.toString();π€ Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| String xmlResponse = restTemplate.exchange(url, HttpMethod.GET, entity, String.class).getBody(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| try { | ||||||||||||||||||||
| NaverApiResponse naverApiResponse = parseBooks(xmlResponse); | ||||||||||||||||||||
| return naverApiResponse.getChannel().getItems(); | ||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||
| e.printStackTrace(); | ||||||||||||||||||||
| throw new XMLParseException("Failed to parse XML response"); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+51
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Improve error handling and logging Several issues with the current error handling:
+import lombok.extern.slf4j.Slf4j;
+@Slf4j
@Component
@RequiredArgsConstructor
public class NaverBookClient implements BookClient{
try {
NaverApiResponse naverApiResponse = parseBooks(xmlResponse);
return naverApiResponse.getChannel().getItems();
- } catch (Exception e) {
- e.printStackTrace();
- throw new XMLParseException("Failed to parse XML response");
+ } catch (JsonProcessingException e) {
+ log.error("Failed to parse XML response from Naver API", e);
+ throw new RuntimeException("Failed to parse XML response from Naver API", e);
}
π€ Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| private NaverApiResponse parseBooks(String xmlResponse) throws Exception { | ||||||||||||||||||||
| XmlMapper xmlMapper = new XmlMapper(); | ||||||||||||||||||||
| return xmlMapper.readValue(xmlResponse, NaverApiResponse.class); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+60
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Reuse XmlMapper instance for better performance Creating a new + private static final XmlMapper xmlMapper = new XmlMapper();
private NaverApiResponse parseBooks(String xmlResponse) throws Exception {
- XmlMapper xmlMapper = new XmlMapper();
return xmlMapper.readValue(xmlResponse, NaverApiResponse.class);
}π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||
| } | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,70 +1,127 @@ | ||
| package org.poolc.api.book.controller; | ||
|
|
||
| import lombok.RequiredArgsConstructor; | ||
| import org.poolc.api.book.dto.BookRequest; | ||
| import org.poolc.api.book.dto.BookResponse; | ||
| import org.poolc.api.book.client.BookClient; | ||
| import org.poolc.api.book.domain.BookSearchOption; | ||
| import org.poolc.api.book.domain.BookSortOption; | ||
| import org.poolc.api.book.dto.request.CreateBookRequest; | ||
| import org.poolc.api.book.dto.request.UpdateBookRequest; | ||
| import org.poolc.api.book.dto.response.BookApiResponse; | ||
| import org.poolc.api.book.dto.response.BookResponse; | ||
| import org.poolc.api.book.service.BookService; | ||
| import org.poolc.api.book.vo.BookCreateValues; | ||
| import org.poolc.api.book.vo.BookUpdateValues; | ||
| import org.poolc.api.member.domain.Member; | ||
| import org.springframework.http.MediaType; | ||
| import org.springframework.data.domain.Page; | ||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.security.core.annotation.AuthenticationPrincipal; | ||
| import org.springframework.web.bind.annotation.*; | ||
|
|
||
| import java.util.HashMap; | ||
| import javax.validation.Valid; | ||
| import javax.validation.constraints.Min; | ||
| import java.util.List; | ||
|
|
||
| import static java.util.stream.Collectors.toList; | ||
|
|
||
| @RestController | ||
| @RequiredArgsConstructor | ||
| @RequestMapping("/book") | ||
| @RequiredArgsConstructor | ||
| public class BookController { | ||
|
|
||
| private final BookClient bookClient; | ||
| private final BookService bookService; | ||
|
|
||
| @GetMapping(value = "/{bookID}", produces = MediaType.APPLICATION_JSON_VALUE) | ||
| public ResponseEntity<BookResponse> findOneBookWithBorrower(@PathVariable("bookID") Long id) { | ||
| return ResponseEntity.ok().body(BookResponse.of(bookService.findOneBook(id))); | ||
| @GetMapping("/naver/search") | ||
| public ResponseEntity<List<BookApiResponse>> searchBooksFromAPI(@RequestParam String query, | ||
| @RequestParam(value = "page", defaultValue = "0") @Min(0) Integer page) { | ||
| try { | ||
| return new ResponseEntity<>(bookClient.searchBooks(query, page), HttpStatus.OK); | ||
| } catch (Exception e) { | ||
| return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } | ||
| } | ||
|
Comment on lines
+31
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Improve error handling with specific HTTP status codes. The generic catch-all exception handling returns 500 for all errors, making it difficult to distinguish between validation errors (400), not found (404), unauthorized (403), etc. Consider using Example approach: @ExceptionHandler(IllegalArgumentException.class)
public ResponseEntity<String> handleBadRequest(IllegalArgumentException e) {
return ResponseEntity.badRequest().body(e.getMessage());
}
@ExceptionHandler(BookNotFoundException.class)
public ResponseEntity<String> handleNotFound(BookNotFoundException e) {
return ResponseEntity.notFound().build();
}Also applies to: 41-52, 54-63, 65-72, 74-83, 85-93, 95-104, 106-114, 116-124 π€ Prompt for AI Agents |
||
|
|
||
| @GetMapping("/search") | ||
| public ResponseEntity<Page<BookResponse>> searchBooks( | ||
| @RequestParam(value = "page", defaultValue = "0") @Min(0) Integer page, | ||
| @RequestParam(value = "search", required = true)BookSearchOption searchOption, | ||
| @RequestParam(value = "keyword", required = true) String keyword, | ||
| @RequestParam(value = "sort", required = false) BookSortOption sortOption) { | ||
| try { | ||
| return new ResponseEntity<>(bookService.searchBooks(page,searchOption,keyword,sortOption), HttpStatus.OK); | ||
| } catch (Exception e) { | ||
| return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } | ||
| } | ||
|
|
||
| @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) | ||
| public ResponseEntity<HashMap<String, List<BookResponse>>> findBooks() { | ||
| HashMap<String, List<BookResponse>> responseBody = new HashMap<>(); | ||
| responseBody.put("data", bookService.findBooks().stream() | ||
| .map(BookResponse::of) | ||
| .collect(toList())); | ||
| return ResponseEntity.ok().body(responseBody); | ||
| @GetMapping("/all") | ||
| public ResponseEntity<Page<BookResponse>> getAllBooks( | ||
| @RequestParam(value = "page", defaultValue = "0") @Min(0) Integer page, | ||
| @RequestParam(value = "sort", required = false) BookSortOption sortOption) { | ||
| try { | ||
| return new ResponseEntity<>(bookService.getAllBooks(page, sortOption), HttpStatus.OK); | ||
| } catch (Exception e) { | ||
| return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } | ||
| } | ||
|
|
||
| @PostMapping(produces = MediaType.APPLICATION_JSON_VALUE) | ||
| public ResponseEntity<Void> registerBook(@RequestBody BookRequest requestBody) { | ||
| bookService.saveBook(new BookCreateValues(requestBody)); | ||
| return ResponseEntity.ok().build(); | ||
| @GetMapping("/{id}") | ||
| public ResponseEntity<BookResponse> getBook(@PathVariable Long id) { | ||
| try { | ||
| return new ResponseEntity<>(bookService.getBook(id), HttpStatus.OK); | ||
| } catch (Exception e) { | ||
| return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } | ||
| } | ||
|
|
||
| @DeleteMapping(value = "/{bookID}", produces = MediaType.APPLICATION_JSON_VALUE) | ||
| public ResponseEntity<Void> deleteBook(@PathVariable("bookID") Long id) { | ||
| bookService.deleteBook(id); | ||
| return ResponseEntity.ok().build(); | ||
| @PostMapping("/new") | ||
| public ResponseEntity<Void> addBook(@AuthenticationPrincipal Member member, | ||
| @Valid @RequestBody CreateBookRequest request) { | ||
| try { | ||
| bookService.createBook(member, request); | ||
| return new ResponseEntity<>(HttpStatus.CREATED); | ||
| } catch (Exception e) { | ||
| return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } | ||
| } | ||
|
|
||
| @PutMapping(value = "/{bookID}", produces = MediaType.APPLICATION_JSON_VALUE) | ||
| public ResponseEntity<Void> updateBook(@RequestBody BookRequest requestBody, @PathVariable("bookID") Long id) { | ||
| bookService.updateBook(id, new BookUpdateValues(requestBody)); | ||
| return ResponseEntity.ok().build(); | ||
| @DeleteMapping("/{id}") | ||
| public ResponseEntity<Void> deleteBook(@AuthenticationPrincipal Member member, @PathVariable Long id) { | ||
| try { | ||
| bookService.deleteBook(member, id); | ||
| return new ResponseEntity<>(HttpStatus.OK); | ||
| } catch (Exception e) { | ||
| return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } | ||
| } | ||
|
|
||
| @PutMapping(value = "/borrow/{bookID}") | ||
| public ResponseEntity borrowBook(@AuthenticationPrincipal Member member, @PathVariable("bookID") Long id) { | ||
| bookService.borrowBook(member, id); | ||
| return ResponseEntity.ok().build(); | ||
| @PutMapping("/{id}") | ||
| public ResponseEntity<Void> updateBook(@AuthenticationPrincipal Member member, @PathVariable Long id, | ||
| @Valid @RequestBody UpdateBookRequest request) { | ||
| try { | ||
| bookService.updateBook(member, id, request); | ||
| return new ResponseEntity<>(HttpStatus.OK); | ||
| } catch (Exception e) { | ||
| return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } | ||
| } | ||
|
|
||
| @PutMapping(value = "/return/{bookID}") | ||
| public ResponseEntity returnBook(@AuthenticationPrincipal Member member, @PathVariable("bookID") Long id) { | ||
| bookService.returnBook(member, id); | ||
| return ResponseEntity.ok().build(); | ||
| @PostMapping("/{id}/borrow") | ||
| public ResponseEntity<Void> borrowBook(@AuthenticationPrincipal Member member, @PathVariable Long id) { | ||
| try { | ||
| bookService.rent(member, id); | ||
| return new ResponseEntity<>(HttpStatus.OK); | ||
| } catch (Exception e) { | ||
| return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @PostMapping("/{id}/return") | ||
| public ResponseEntity<Void> returnBook(@AuthenticationPrincipal Member member, @PathVariable Long id) { | ||
| try { | ||
| bookService.returnBook(member, id); | ||
| return new ResponseEntity<>(HttpStatus.OK); | ||
| } catch (Exception e) { | ||
| return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use appropriate exception type for XML parsing.
The import
javax.management.modelmbean.XMLParseExceptionis incorrect for general XML parsing. This exception is specifically for JMX management beans, not for parsing XML responses from external APIs.Consider using a more appropriate exception type:
And update the method signature:
Alternatively, you could use a more general exception like
Exceptionor create a custom exception class for book client operations.π€ Prompt for AI Agents