-
Notifications
You must be signed in to change notification settings - Fork 291
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
fix: [BREAKING] remove legacy subscription; change subscription interface #394
base: master
Are you sure you want to change the base?
Conversation
Hey sorry I plan to review this this weekend, thanks! |
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.
Thanks! Sorry for the delay, I'm not sure about the change about null => ""
though
$associativeArray['publicKey'] ?? null, | ||
$associativeArray['authToken'] ?? null, | ||
$associativeArray['contentEncoding'] ?? "aesgcm" | ||
$associativeArray['endpoint'] ?? "", |
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.
endpoint shouldn't be empty, it should throw before, shouldn't it?
$associativeArray['authToken'] ?? null, | ||
$associativeArray['contentEncoding'] ?? "aesgcm" | ||
$associativeArray['endpoint'] ?? "", | ||
$associativeArray['keys']['p256dh'] ?? "", |
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'm not sure this is an improvement, having an empty string instead of null ?
* @author Sergii Bondarenko <sb@firstvector.org> | ||
*/ | ||
interface SubscriptionInterface | ||
{ | ||
public function getEndpoint(): string; | ||
|
||
public function getPublicKey(): ?string; | ||
public function getPublicKey(): string; |
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.
A subscription can have no public key, auth token or content encoding (sent without any payload)
Remove legacy GCM Remove old Chrome subscription support
[BREAKING] change default encoding to aes128gcm
e632f72
to
59f2e6e
Compare
Changes
Breaking:
Feature:
Reference
fixes #381
fixes #345
reference #388