Skip to content

Commit 41cb49e

Browse files
SecondeJKSecondeJK
authored
GSM-7 warning handling for users (#351)
* Probably the best solution I could come up with to make sure that users are using the right encoding * test for sending unicode message as text * Property types, split out error message handling to make it cleaner with inheritance
1 parent 3258052 commit 41cb49e

File tree

10 files changed

+124
-62
lines changed

10 files changed

+124
-62
lines changed

‎src/SMS/Client.php‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Vonage\Client\Exception\Exception as ClientException;
1818
use Vonage\Client\Exception\ThrottleException;
1919
use Vonage\SMS\Message\Message;
20+
use Vonage\SMS\Message\SMS;
2021

2122
use function sleep;
2223

@@ -31,22 +32,15 @@ public function getAPIResource(): APIResource
3132
return $this->api;
3233
}
3334

34-
public function isUnicode($message): bool
35-
{
36-
return strlen($message) !== strlen(mb_convert_encoding($message, 'ISO-8859-1', 'UTF-8'));
37-
}
38-
3935
/**
4036
* @throws ClientExceptionInterface
4137
* @throws ClientException
4238
*/
4339
public function send(Message $message): Collection
4440
{
45-
if (($message->getType() === 'text') && $this->isUnicode($message->getMessage())) {
41+
if ($message->getErrorMessage()) {
4642
trigger_error(
47-
"Sending unicode text SMS without setting the type parameter to 'unicode'.
48-
See https://developer.vonage.com/messaging/sms for details, or email support@vonage.com
49-
if you have any questions.",
43+
$message->getErrorMessage(),
5044
E_USER_WARNING
5145
);
5246
}

‎src/SMS/Message/Binary.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class Binary extends OutboundMessage
1616
/**
1717
* @var string
1818
*/
19-
protected $type = 'binary';
19+
protected string $type = 'binary';
2020

2121
public function __construct(string $to, string $from, protected string $body, protected string $udh, protected ?int $protocolId = null)
2222
{

‎src/SMS/Message/Message.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,6 @@
1414
interface Message
1515
{
1616
public function toArray(): array;
17+
public function getErrorMessage(): ?string;
18+
public function setErrorMessage(?string $errorMessage): void;
1719
}

‎src/SMS/Message/OutboundMessage.php‎

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,30 +19,15 @@
1919

2020
abstract class OutboundMessage implements Message
2121
{
22-
/**
23-
* @var ?string
24-
*/
25-
protected $accountRef;
22+
protected ?string $accountRef = null;
2623

27-
/**
28-
* @var string
29-
*/
30-
protected $clientRef;
24+
protected ?string $clientRef = null;
3125

32-
/**
33-
* @var ?string
34-
*/
35-
protected $deliveryReceiptCallback;
26+
protected ?string $deliveryReceiptCallback = null;
3627

37-
/**
38-
* @var int
39-
*/
40-
protected $messageClass;
28+
protected ?int $messageClass = null;
4129

42-
/**
43-
* @var bool
44-
*/
45-
protected $requestDeliveryReceipt = true;
30+
protected bool $requestDeliveryReceipt = true;
4631

4732
/**
4833
* TTL of the SMS delivery, in milliseconds
@@ -51,17 +36,29 @@ abstract class OutboundMessage implements Message
5136
*/
5237
protected $ttl = 259200000;
5338

39+
protected ?string $errorMessage = null;
40+
5441
/**
5542
* Type of message, set by the child class
5643
*
5744
* @var string
5845
*/
59-
protected $type;
46+
protected string $type;
6047

6148
public function __construct(protected string $to, protected string $from)
6249
{
6350
}
6451

52+
public function getErrorMessage(): ?string
53+
{
54+
return $this->errorMessage;
55+
}
56+
57+
public function setErrorMessage(?string $errorMessage): void
58+
{
59+
$this->errorMessage = $errorMessage;
60+
}
61+
6562
abstract public function toArray(): array;
6663

6764
public function getTtl(): int

‎src/SMS/Message/SMS.php‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,27 +13,28 @@
1313

1414
class SMS extends OutboundMessage
1515
{
16-
/**
17-
* @var string
18-
*/
19-
protected $contentId;
16+
public const GSM_7_PATTERN = '/\A[\n\f\r !\"\#$%&\'()*+,-.\/0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\\\\^_abcdefghijklmnopqrstuvwxyz{\|}~ ¡£¤¥§¿ÄÅÆÇÉÑÖØÜßàäåæèéìñòöøùüΓΔΘΛΞΠΣΦΨΩ€]*\z/m';
2017

21-
/**
22-
* @var string
23-
*/
24-
protected $entityId;
18+
protected ?string $contentId;
19+
20+
protected ?string $entityId;
2521

2622
/**
2723
* @var string
2824
*/
29-
protected $type = 'unicode';
25+
protected string $type = 'text';
3026

31-
public function __construct(string $to, string $from, protected string $message, string $type = 'unicode')
27+
public function __construct(string $to, string $from, protected string $message, string $type = 'text')
3228
{
3329
parent::__construct($to, $from);
3430
$this->setType($type);
3531
}
3632

33+
public function isGsm7(): bool
34+
{
35+
return (bool)preg_match(self::GSM_7_PATTERN, $this->getMessage());
36+
}
37+
3738
public function getContentId(): string
3839
{
3940
return $this->contentId;
@@ -56,6 +57,25 @@ public function setEntityId(string $id): self
5657
return $this;
5758
}
5859

60+
public function getErrorMessage(): ?string
61+
{
62+
if ($this->getType() === 'unicode' && $this->isGsm7()) {
63+
$this->setErrorMessage("You are sending a message as `unicode` when it could be `text` or a
64+
`text` type with unicode-only characters. This could result in increased billing -
65+
See https://developer.vonage.com/messaging/sms for details, or email support@vonage.com if you have any
66+
questions.");
67+
}
68+
69+
if ($this->getType() === 'text' && ! $this->isGsm7()) {
70+
$this->setErrorMessage("You are sending a message as `text` when contains unicode only
71+
characters. This could result in encoding problems with the target device or increased billing - See
72+
https://developer.vonage.com/messaging/sms for details, or email support@vonage.com if you have any
73+
questions.");
74+
}
75+
76+
return $this->errorMessage;
77+
}
78+
5979
/**
6080
* @return array<mixed>
6181
*/

‎src/SMS/Message/Vcal.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class Vcal extends OutboundMessage
1616
/**
1717
* @var string
1818
*/
19-
protected $type = 'vcal';
19+
protected string $type = 'vcal';
2020

2121
public function __construct(string $to, string $from, protected string $event)
2222
{

‎src/SMS/Message/Vcard.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class Vcard extends OutboundMessage
1616
/**
1717
* @var string
1818
*/
19-
protected $type = 'vcard';
19+
protected string $type = 'vcard';
2020

2121
public function __construct(string $to, string $from, protected string $card)
2222
{

‎src/SMS/Message/WAPPush.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class WAPPush extends OutboundMessage
1616
/**
1717
* @var string
1818
*/
19-
protected $type = 'wappush';
19+
protected string $type = 'wappush';
2020

2121
public function __construct(string $to, string $from, protected string $title, protected string $url, protected int $validity)
2222
{

‎test/SMS/ClientTest.php‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -377,29 +377,57 @@ public function testCanHandleMissingShortcodeOn2FA(): void
377377
$this->smsClient->sendTwoFactor('447700900000', 1245);
378378
}
379379

380-
public function testThrowsWarningSendingUnicodeAsTest(): void
380+
public function testThrowsWarningWhenSendingTextAsUnicode(): void
381381
{
382382
$this->vonageClient->send(Argument::that(function (Request $request) {
383383
return true;
384384
}))->willReturn($this->getResponse('send-success'));
385385

386386
$this->expectWarning();
387-
$this->expectErrorMessage("Sending unicode text SMS without setting the type parameter to 'unicode'.
388-
See https://developer.vonage.com/messaging/sms for details, or email support@vonage.com
389-
if you have any questions.");
387+
$this->expectErrorMessage("You are sending a message as `unicode` when it could be `text` or a
388+
`text` type with unicode-only characters. This could result in increased billing -
389+
See https://developer.vonage.com/messaging/sms for details, or email support@vonage.com if you have any
390+
questions.");
390391

391392
$args = [
392393
'to' => '447700900000',
393394
'from' => '16105551212',
394-
'text' => "♗ⳋ⤞ⶢⲍ⫓⵬⬕⫹⿁⊅⇧ⰾ⾵ ⃚",
395+
'text' => "This Can Be Sent As GSM7",
395396
'account-ref' => 'customer1234',
396397
'client-ref' => 'my-personal-reference'
397398
];
398399

399400
$message = (new SMS($args['to'], $args['from'], $args['text']))
400401
->setClientRef($args['client-ref'])
401402
->setAccountRef($args['account-ref'])
402-
->setType('text');
403+
->setType('unicode');
404+
405+
$response = $this->smsClient->send($message);
406+
}
407+
408+
public function testThrowsWarningWhenSendingUnicodeAsText(): void
409+
{
410+
$this->vonageClient->send(Argument::that(function (Request $request) {
411+
return true;
412+
}))->willReturn($this->getResponse('send-success'));
413+
414+
$this->expectWarning();
415+
$this->expectErrorMessage("You are sending a message as `text` when contains unicode only
416+
characters. This could result in encoding problems with the target device or increased billing - See
417+
https://developer.vonage.com/messaging/sms for details, or email support@vonage.com if you have any
418+
questions.");
419+
420+
$args = [
421+
'to' => '447700900000',
422+
'from' => '16105551212',
423+
'text' => "This needs to be sent as unicode これはユニコード",
424+
'account-ref' => 'customer1234',
425+
'client-ref' => 'my-personal-reference'
426+
];
427+
428+
$message = (new SMS($args['to'], $args['from'], $args['text']))
429+
->setClientRef($args['client-ref'])
430+
->setAccountRef($args['account-ref']);
403431

404432
$response = $this->smsClient->send($message);
405433
}

‎test/SMS/Message/SMSTest.php‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,15 @@ class SMSTest extends VonageTestCase
2020
public function testCanSetUnicodeType(): void
2121
{
2222
$sms = (new SMS('447700900000', '16105551212', 'Test Message'));
23-
$this->assertSame('unicode', $sms->getType());
24-
$sms->setType('text');
2523
$this->assertSame('text', $sms->getType());
24+
$sms->setType('unicode');
25+
$this->assertSame('unicode', $sms->getType());
2626
}
2727

2828
public function testCanSetUnicodeTypeInConstructor(): void
2929
{
30-
$sms = (new SMS('447700900000', '16105551212', 'Test Message', 'text'));
31-
$this->assertSame('text', $sms->getType());
30+
$sms = (new SMS('447700900000', '16105551212', 'Test Message', 'unicode'));
31+
$this->assertSame('unicode', $sms->getType());
3232
}
3333

3434
public function testDeliveryCallbackCanBeSet(): void
@@ -96,7 +96,7 @@ public function testCannotSetTooLongOfaClientRef(): void
9696
->setClientRef('This is a really long client ref and should throw an exception');
9797
}
9898

99-
public function testCanSetEntityId()
99+
public function testCanSetEntityId(): void
100100
{
101101
$sms = new SMS('447700900000', '16105551212', 'Test Message');
102102
$sms->setEntityId('abcd');
@@ -106,7 +106,7 @@ public function testCanSetEntityId()
106106
'entity-id' => 'abcd',
107107
'to' => '447700900000',
108108
'from' => '16105551212',
109-
'type' => 'unicode',
109+
'type' => 'text',
110110
'ttl' => 259200000,
111111
'status-report-req' => 1,
112112
];
@@ -115,7 +115,7 @@ public function testCanSetEntityId()
115115
$this->assertSame($expected['entity-id'], $sms->getEntityId());
116116
}
117117

118-
public function testCanSetContentId()
118+
public function testCanSetContentId(): void
119119
{
120120
$sms = new SMS('447700900000', '16105551212', 'Test Message');
121121
$sms->setContentId('1234');
@@ -125,7 +125,7 @@ public function testCanSetContentId()
125125
'content-id' => '1234',
126126
'to' => '447700900000',
127127
'from' => '16105551212',
128-
'type' => 'unicode',
128+
'type' => 'text',
129129
'ttl' => 259200000,
130130
'status-report-req' => 1,
131131
];
@@ -134,7 +134,7 @@ public function testCanSetContentId()
134134
$this->assertSame($expected['content-id'], $sms->getContentId());
135135
}
136136

137-
public function testDLTInfoAppearsInRequest()
137+
public function testDLTInfoAppearsInRequest(): void
138138
{
139139
$sms = new SMS('447700900000', '16105551212', 'Test Message');
140140
$sms->enableDLT('abcd', '1234');
@@ -145,27 +145,48 @@ public function testDLTInfoAppearsInRequest()
145145
'content-id' => '1234',
146146
'to' => '447700900000',
147147
'from' => '16105551212',
148-
'type' => 'unicode',
148+
'type' => 'text',
149149
'ttl' => 259200000,
150150
'status-report-req' => 1,
151151
];
152152

153153
$this->assertSame($expected, $sms->toArray());
154154
}
155155

156-
public function testDLTInfoDoesNotAppearsWhenNotSet()
156+
public function testDLTInfoDoesNotAppearsWhenNotSet(): void
157157
{
158158
$sms = new SMS('447700900000', '16105551212', 'Test Message');
159159

160160
$expected = [
161161
'text' => 'Test Message',
162162
'to' => '447700900000',
163163
'from' => '16105551212',
164-
'type' => 'unicode',
164+
'type' => 'text',
165165
'ttl' => 259200000,
166166
'status-report-req' => 1,
167167
];
168168

169169
$this->assertSame($expected, $sms->toArray());
170170
}
171+
172+
/**
173+
* @dataProvider unicodeStringDataProvider
174+
* @return void
175+
*/
176+
public function testGsm7Identification(string $message, bool $expectedGsm7)
177+
{
178+
$sms = new SMS('16105551212', '16105551212', $message);
179+
$this->assertEquals($expectedGsm7, $sms->isGsm7());
180+
}
181+
182+
public function unicodeStringDataProvider(): array
183+
{
184+
return [
185+
['this is a text', true],
186+
['This is also a GSM7 text', true],
187+
['This is a Çotcha', true],
188+
['This is also a çotcha', true],
189+
['日本語でボナージュ', false],
190+
];
191+
}
171192
}

0 commit comments

Comments
 (0)