Skip to content
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

Feat/sms batches: Send & DryRun #2

Merged
merged 3 commits into from
Nov 3, 2023
Merged

Feat/sms batches: Send & DryRun #2

merged 3 commits into from
Nov 3, 2023

Conversation

JPPortier
Copy link
Contributor

  • Implement SMS batches Send and DryRun
  • Merge/PR for sms branches and not main: sms feature branch will be merge to main only when full set of SMS features will be available

Copy link
Contributor

github-actions bot commented Nov 3, 2023

Dependency Review

✅ No vulnerabilities or license issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 5ca7808.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Manifest Files

private final int maxNumberOfMessageParts;
private final int fromTon;
private final int fromNpi;
private final Boolean truncateConcat;

Choose a reason for hiding this comment

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

Why do you need to use Objects instead of primitives?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to autoboxing.
When receiving a DTO instance where fields is not present or null: autoboxing is causing NullPointerException to map missing value to primitive.
e.g.: Sending a SMS with only toand body field (see sample-app Send source)
REST API is returning:

{
    "id": "01HEAC0AG69SVYYQ675VPYT28Q",
    "to": [
        "XXXXX"
    ],
    "canceled": false,
    "body": "the body",
    "type": "mt_text",
    "created_at": "2023-11-03T10:35:03.558Z",
    "modified_at": "2023-11-03T10:35:03.558Z",
    "delivery_report": "none",
    "expire_at": "2023-11-06T10:35:03.558Z",
    "feedback_enabled": false,
    "flash_message": false
}

Then BatchText.truncatConcat as boolean primitive cannot contains "valid" information (nor true, nor false) and autoboxing during convertion from DTO(https://github.com/sinch/sinch-sdk-java/pull/2/files#diff-45ba5a02d31c9bfab5d5e2a1c03fdf3809bc78697f9dcbbffbc9c0bc6652f4a2R110) is throwing the exception because:

  • dto.getTruncateConcat() is returning null (field is a Boolean and not boolean)
  • then null cannot be converted to a boolean because of null value
  • then NullPointerException

Instead of modifying DTO object to use primitives, I'm using Object to be able have distinction between what was recevied/set and what was not.
In addition if there is some default value to be set; then it is to the server side to set this default value on return.

Copy link

@asein-sinch asein-sinch left a comment

Choose a reason for hiding this comment

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

It's nice to see generated files updated after you changed the mustache template!

@JPPortier JPPortier merged commit b7c53c6 into sms Nov 3, 2023
2 checks passed
@JPPortier JPPortier deleted the feat/sms-batches branch November 3, 2023 11:51
JPPortier added a commit that referenced this pull request Nov 16, 2023
* feat (DEVEXP-166): SMS / Batches: Send & DryRun
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