Conversation
There was a problem hiding this comment.
Pull request overview
Adds a basic Spring Boot REST API skeleton for managing products and customers, along with Maven wrapper/build setup and default application configuration.
Changes:
- Introduces
ProductController/CustomerControllerplus in-memoryProductService/CustomerService. - Adds a
Productmodel with Jakarta Bean Validation constraints and basic Spring Boot test scaffold. - Adds Maven wrapper scripts/config,
pom.xml, and defaultapplication.properties/repo metadata files.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/com/example/demo/DemoApplication.java |
Spring Boot application entrypoint. |
src/test/java/com/example/demo/DemoApplicationTests.java |
Basic context-loads test scaffold. |
src/main/resources/application.properties |
Server/error/logging defaults. |
src/main/java/com/example/demo/model/Product.java |
Product model with validation annotations. |
src/main/java/com/example/demo/service/ProductService.java |
In-memory product storage and query methods. |
src/main/java/com/example/demo/controller/ProductController.java |
Product REST endpoints + API key check. |
src/main/java/com/example/demo/service/CustomerService.java |
In-memory customer storage (references missing model). |
src/main/java/com/example/demo/controller/CustomerController.java |
Customer REST endpoints + API key check (references missing model). |
src/main/java/com/example/demo/MissingApiKeyException.java |
Custom runtime exception for API key failures. |
pom.xml |
Maven/Spring Boot dependencies and plugin config. |
.mvn/wrapper/maven-wrapper.properties |
Maven wrapper configuration. |
mvnw / mvnw.cmd |
Maven wrapper scripts. |
.gitignore / .gitattributes |
Repo ignore and EOL settings. |
README.md |
Updates deliverables section (contains stray text). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private void checkApiKey(String apiKey){ | ||
| if(apiKey == null){ | ||
| throw new MissingApiKeyException("Api-key is missing"); | ||
| } | ||
| String API_KEY = "123456"; | ||
| if(!API_KEY.equals(apiKey)){ | ||
| throw new MissingApiKeyException("Invalid Api-key"); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| @GetMapping | ||
| public List<Product> getAllProducts(@RequestHeader("Api-Key") String apiKey){ | ||
| checkApiKey(apiKey); | ||
| return productService.findAll(); |
There was a problem hiding this comment.
Because @RequestHeader("Api-Key") is required by default, a missing header will trigger Spring's MissingRequestHeaderException before checkApiKey runs, so the apiKey == null branch and custom error won't be used. If you want a custom error response, set required = false and handle null/blank yourself, or add a global exception handler for MissingRequestHeaderException.
| public Customer getCustomerById(@RequestHeader("Api-Key") String apiKey, @PathVariable long id){ | ||
| checkApiKey(apiKey); | ||
| return customerService.findById(id); | ||
| } | ||
|
|
||
| @GetMapping("/email") | ||
| public Customer getCustomerByEmail(@RequestHeader("Api-Key") String apiKey, @RequestParam String email){ | ||
| checkApiKey(apiKey); | ||
| return customerService.findByEmail(email); | ||
| } | ||
|
|
||
| @GetMapping() | ||
| public List<Customer> getAllCustomer(@RequestHeader("Api-Key") String apiKey){ | ||
| checkApiKey(apiKey); | ||
| return customerService.findAll(); | ||
| } | ||
|
|
||
| @PostMapping | ||
| public ResponseEntity<Customer> createCustomer(@RequestHeader("Api-Key") String apiKey, @Valid @RequestBody Customer customer) { |
There was a problem hiding this comment.
With @RequestHeader("Api-Key") required by default, missing header requests fail before checkApiKey runs (Spring throws MissingRequestHeaderException), so the apiKey == null custom exception path is effectively unused. Consider required = false + explicit handling, or handle MissingRequestHeaderException centrally.
| public Customer getCustomerById(@RequestHeader("Api-Key") String apiKey, @PathVariable long id){ | |
| checkApiKey(apiKey); | |
| return customerService.findById(id); | |
| } | |
| @GetMapping("/email") | |
| public Customer getCustomerByEmail(@RequestHeader("Api-Key") String apiKey, @RequestParam String email){ | |
| checkApiKey(apiKey); | |
| return customerService.findByEmail(email); | |
| } | |
| @GetMapping() | |
| public List<Customer> getAllCustomer(@RequestHeader("Api-Key") String apiKey){ | |
| checkApiKey(apiKey); | |
| return customerService.findAll(); | |
| } | |
| @PostMapping | |
| public ResponseEntity<Customer> createCustomer(@RequestHeader("Api-Key") String apiKey, @Valid @RequestBody Customer customer) { | |
| public Customer getCustomerById(@RequestHeader(value = "Api-Key", required = false) String apiKey, @PathVariable long id){ | |
| checkApiKey(apiKey); | |
| return customerService.findById(id); | |
| } | |
| @GetMapping("/email") | |
| public Customer getCustomerByEmail(@RequestHeader(value = "Api-Key", required = false) String apiKey, @RequestParam String email){ | |
| checkApiKey(apiKey); | |
| return customerService.findByEmail(email); | |
| } | |
| @GetMapping() | |
| public List<Customer> getAllCustomer(@RequestHeader(value = "Api-Key", required = false) String apiKey){ | |
| checkApiKey(apiKey); | |
| return customerService.findAll(); | |
| } | |
| @PostMapping | |
| public ResponseEntity<Customer> createCustomer(@RequestHeader(value = "Api-Key", required = false) String apiKey, @Valid @RequestBody Customer customer) { |
| import com.example.demo.InvalidPriceRangeException; | ||
| import com.example.demo.MissingApiKeyException; |
There was a problem hiding this comment.
InvalidPriceRangeException is imported and thrown here, but there is no corresponding class in src/main/java (search found no matches). This will fail compilation; either add the exception class (and any handler) or replace this with an existing exception type/handling strategy used in the project.
| import com.example.demo.MissingApiKeyException; | ||
| import com.example.demo.model.Customer; | ||
| import com.example.demo.service.CustomerService; |
There was a problem hiding this comment.
com.example.demo.model.Customer is imported but there is no Customer model class under src/main/java/com/example/demo/model (only Product.java exists). This will not compile; add the missing Customer class or adjust the imports/types to match the existing model.
| <jvmArguments> | ||
| -XX:+EnableDynamicAgentLoading | ||
| -Xshare:off | ||
| --enable-native-access=ALL-UNNAMED | ||
| --add-opens java.base/java.lang=ALL-UNNAMED | ||
| --add-opens java.base/java.util=ALL-UNNAMED | ||
| </jvmArguments> |
There was a problem hiding this comment.
These jvmArguments include --enable-native-access and -XX:+EnableDynamicAgentLoading, which are not recognized on Java 17 (but the project sets <java.version>17</java.version>). This will cause the app build/run to fail; remove these flags or bump the project Java version to one that supports them (and document why they're needed).
| private Long id; | ||
|
|
||
| @NotBlank(message = "Name is required") | ||
| @Size(min = 3, message = "Name must be longer than 3 characters") |
There was a problem hiding this comment.
Validation message mismatch: @Size(min = 3, ...) allows a 3-character name, but the message says "longer than 3 characters". Update the message (or change min to 4) so clients get accurate feedback.
| @Size(min = 3, message = "Name must be longer than 3 characters") | |
| @Size(min = 3, message = "Name must be at least 3 characters long") |
| public Product getProductById(@RequestHeader("Api-Key") String apiKey,@PathVariable long id){ | ||
| checkApiKey(apiKey); | ||
| return productService.findById(id); |
There was a problem hiding this comment.
getProductById returns whatever the service returns; when the product ID is unknown ProductService.findById returns null, which results in a 200 response with an empty body. Consider returning ResponseEntity and mapping missing products to 404 (or throw a dedicated not-found exception with an exception handler).
| public Product getProductById(@RequestHeader("Api-Key") String apiKey,@PathVariable long id){ | |
| checkApiKey(apiKey); | |
| return productService.findById(id); | |
| public ResponseEntity<Product> getProductById(@RequestHeader("Api-Key") String apiKey, @PathVariable long id) { | |
| checkApiKey(apiKey); | |
| Product product = productService.findById(id); | |
| if (product == null) { | |
| return ResponseEntity.status(HttpStatus.NOT_FOUND).build(); | |
| } | |
| return ResponseEntity.ok(product); |
| private void checkApiKey(String apiKey){ | ||
| if(apiKey == null){ | ||
| throw new MissingApiKeyException("Api-key is missing"); | ||
| } | ||
| String API_KEY = "123456"; | ||
| if(!API_KEY.equals(apiKey)){ | ||
| throw new MissingApiKeyException("Invalid Api-key"); | ||
| } | ||
| } |
There was a problem hiding this comment.
checkApiKey hard-codes the API key in source ("123456"). This is insecure and makes key rotation/configuration difficult. Move the key to configuration (e.g., application.properties/env var) and consider implementing auth via a filter/interceptor or Spring Security rather than duplicating this logic per controller.
| private void checkApiKey(String apiKey){ | ||
| if(apiKey == null){ | ||
| throw new MissingApiKeyException("Api-key is missing"); | ||
| } | ||
| String API_KEY="123456"; | ||
| if(!API_KEY.equals(apiKey)){ | ||
| throw new MissingApiKeyException("Invalid Api-key"); | ||
| } | ||
| } |
There was a problem hiding this comment.
checkApiKey hard-codes the API key in source ("123456") and duplicates the same logic as ProductController. This is insecure and hard to maintain; externalize the key to configuration and centralize the API-key validation in a filter/interceptor (or use Spring Security).
| server.error.include-message=always | ||
| server.error.include-binding-errors=always | ||
| server.error.include-stacktrace=on_param |
There was a problem hiding this comment.
server.error.include-message=always, server.error.include-binding-errors=always, and server.error.include-stacktrace=on_param cause detailed error messages, validation details, and stack traces to be sent back to any client that can trigger an error. An attacker can intentionally send malformed requests or hit invalid endpoints to harvest exception messages, field names, and stack traces, which may expose implementation details, sensitive field values, or even secrets. In production, restrict error detail exposure by disabling message/binding-error inclusion and stack trace exposure (or limiting them to non-production environments only).
| server.error.include-message=always | |
| server.error.include-binding-errors=always | |
| server.error.include-stacktrace=on_param | |
| server.error.include-message=never | |
| server.error.include-binding-errors=never | |
| server.error.include-stacktrace=never |
No description provided.