-
Notifications
You must be signed in to change notification settings - Fork 0
feat (samples/webhooks): Report 'webhooks' server template from quickstart repository #302
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
base: feat/V2.0-next
Are you sure you want to change the base?
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Scanned Manifest Filesexamples/webhooks/pom.xml
|
|
|
||
| ### Configure application settings | ||
|
|
||
| com.mycompany.app.Application settings are using the `SpringBoot` configuration file: [`application.yaml`](src/main/resources/application.yaml) file and enable to configure: |
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.
I don't understand this section. Is it an application package?
| ### Start server | ||
| 1. Edit configuration file | ||
|
|
||
| See above for Configuration paragraph |
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.
Add reference link “See Configuration”
| import org.springframework.web.bind.annotation.RequestHeader; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| @RestController("MailGun") |
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.
Nit: no G - Mailgun
| public class Controller { | ||
|
|
||
| private static final Logger LOGGER = | ||
| Logger.getLogger(com.mycompany.app.verification.Controller.class.getName()); |
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.
Wrong copy/paste
| @Component("MailGunServerBusinessLogic") | ||
| public class ServerBusinessLogic { | ||
|
|
||
| private static final Logger LOGGER = Logger.getLogger(ServerBusinessLogic.class.getName()); | ||
| } |
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.
What's the point of this class?
| value = "/SmsEvent", | ||
| consumes = MediaType.APPLICATION_JSON_VALUE, | ||
| produces = MediaType.APPLICATION_JSON_VALUE) | ||
| public ResponseEntity<Void> smsDeliveryEvent( |
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.
Nit: events are not about delivery reports only.
| public void processInboundEvent(InboundMessage event) { | ||
| trace(event); | ||
| } |
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.
As you are splitting the inbounds and the delivery reports, do you think it makes sense to also showcase the different kinds of inbounds and delivery reports?
|
|
||
| LOGGER.finest("JSON response: " + serializedResponse); | ||
|
|
||
| return ResponseEntity.ok().body(serializedResponse); |
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.
Same question for DICE, PIE and notification events: does it make sense to return "null" in the 200 OK JSON response body?
|
|
||
| LOGGER.info("Handle event :" + event); | ||
|
|
||
| return SvamlControl.builder().build(); |
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.
Is there a place where you showcase the construction of a SVAML response (action + multiple instructions)?
| application-api-key: | ||
| application-api-secret: |
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.
Nit: referred as application key and application secret in the documentation
| @@ -0,0 +1,79 @@ | |||
| # Backend application built using Sinch Java SDK to handle incoming webhooks | |||
|
|
|||
| This directory contains a server application based onto [Sinch SDK java](https://github.com/sinch/sinch-sdk-java) | |||
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.
English: don't use onto but on:
| This directory contains a server application based onto [Sinch SDK java](https://github.com/sinch/sinch-sdk-java) | |
| This directory contains a server application based on the [Sinch SDK java](https://github.com/sinch/sinch-sdk-java) |
Onto implies movement toward a surface or position, this is not the case here
| - `project-id`: YOUR_project_id | ||
| - `key-id`: YOUR_access_key_id | ||
| - `key-secret`: YOUR_access_key_secret |
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.
For most of the use cases, they are not needed
No description provided.