feature (template/server): Provide a server side application template for webhooks #10
Conversation
|
Overall, this is really good stuff. What are your thoughts on how the snippets will be slotted in? Will the snippet file just be able to replace the business logic file, like the way the client app works? |
There was a problem hiding this comment.
Suggestion of class naming: ParsedEventHandler
There was a problem hiding this comment.
According to our last discussion, ServerBusinessLogic will be used
…e classes processing webhooks business logic
| NumberEvent event = webhooks.parseEvent(body); | ||
|
|
||
| // let business layer process the request | ||
| webhooksBusinessLogic.numbersEvent(event); |
There was a problem hiding this comment.
Although thanks to the Spring Boot configuration (typically the @ResponseBody annotation included with @RestController) and the method returning void, this will return an empty body with a status 200 (which is expected), it might be clearer to explicitly a ResponseEntity to make the intended behavior clear
There was a problem hiding this comment.
Very good [educational] idea 👍
Applied to all controllers
| WebHooksService webhooks = sinchClient.numbers().v1().webhooks(); | ||
|
|
||
| // decode the request payload | ||
| NumberEvent event = webhooks.parseEvent(body); |
There was a problem hiding this comment.
Ideally, the incoming request should be verified before being processed
There was a problem hiding this comment.
Right.
Created a ticket to add this missing feature to webhooks service
Thank you to highlight this
| com: INFO | ||
|
|
||
| server: | ||
| port: 8080 |
There was a problem hiding this comment.
This is not the same value as in the README file (8090)
PR based onto Numbers related one because of using Numbers versioned V1; but have to be merged after Numbers to main