Inject UrlBuilder instance into the BigBlueButton constructor #286
Conversation
| $this->hashingAlgorithm = $hashingAlgorithm; | ||
| $this->bbbBaseUrl = $urlBuilder->getBaseUrl(); | ||
| $this->bbbSecret = $urlBuilder->getSecret(); | ||
| $this->hashingAlgorithm = $urlBuilder->getHashingAlgorithm(); |
There was a problem hiding this comment.
We need these getters if we want to fill the deprecated properties.
We can remove all of this in the next major version.
There was a problem hiding this comment.
Actually, maybe we can leave the properties uninitialized?
Any code that currently uses these properties would do so by extending BigBlueButton.
It is unlikely that the same code would also want to inject a UrlBuilder class.
Unless we are talking about two separate packages.
| #[\SensitiveParameter] | ||
| ?string $secret = null, | ||
| ?array $opts = [], | ||
| ?UrlBuilder $urlBuilder = null, |
There was a problem hiding this comment.
For full BC support, the parameter must be appended. We cannot drop or replace the other parameters.
For a smooth way to the next major version, without disruptive BC breaks, we can (in a follow-up) deprecate the constructor and recommend to use dedicated static factory methods.
| // constructor signature. | ||
| // @phpstan-ignore new.static | ||
| return new static($secret, $baseUrl, $hashingAlgorithm); | ||
| } |
There was a problem hiding this comment.
We need to decide whether the env vars should be still supported in the next major version.
Until that is decided, this static factory can remain @internal.
src/BigBlueButton.php
Outdated
| $hashingAlgorithm = HashingAlgorithm::SHA_256; | ||
| public function __construct( | ||
| ?string $baseUrl = null, | ||
| #[\SensitiveParameter] |
There was a problem hiding this comment.
The #[SensitiveParameter] is from a different PR, which we should do first:
donquixote
force-pushed
the
inject-url-builder
branch
from
650b543 to
0b73ff4
Compare
For now I am proposing a version that does not break BC.