-
Notifications
You must be signed in to change notification settings - Fork 202
Inject UrlBuilder instance into the BigBlueButton constructor #286
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: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,35 +92,20 @@ class BigBlueButton | |
/** | ||
* @param null|array<string, mixed> $opts | ||
*/ | ||
public function __construct(?string $baseUrl = null, ?string $secret = null, ?array $opts = []) | ||
{ | ||
// Provide an early error message if configuration is wrong | ||
if (is_null($baseUrl) && false === getenv('BBB_SERVER_BASE_URL')) { | ||
throw new \RuntimeException('No BBB-Server-Url found! Please provide it either in constructor ' | ||
. "(1st argument) or by environment variable 'BBB_SERVER_BASE_URL'!"); | ||
} | ||
|
||
if (is_null($secret) && false === getenv('BBB_SECRET') && false === getenv('BBB_SECURITY_SALT')) { | ||
throw new \RuntimeException('No BBB-Secret (or BBB-Salt) found! Please provide it either in constructor ' | ||
. "(2nd argument) or by environment variable 'BBB_SECRET' (or 'BBB_SECURITY_SALT')!"); | ||
} | ||
|
||
// Keeping backward compatibility with older deployed versions | ||
// BBB_SECRET is the new variable name and have higher priority against the old named BBB_SECURITY_SALT | ||
// Reminder: getenv() will return FALSE if not set. But bool is not accepted by $this->bbbSecret | ||
// nor $this->bbbBaseUrl (only strings), thus FALSE will be converted automatically to an empty | ||
// string (''). Having a bool should be not possible due to the checks above and the automated | ||
// conversion, but PHPStan is still unhappy, so it's covered explicit by adding `?: ''`. | ||
$bbbBaseUrl = $baseUrl ?: getenv('BBB_SERVER_BASE_URL') ?: ''; | ||
$bbbSecret = $secret ?: getenv('BBB_SECRET') ?: getenv('BBB_SECURITY_SALT') ?: ''; | ||
$hashingAlgorithm = HashingAlgorithm::SHA_256; | ||
public function __construct( | ||
?string $baseUrl = null, | ||
?string $secret = null, | ||
?array $opts = [], | ||
?UrlBuilder $urlBuilder = null, | ||
) { | ||
$urlBuilder ??= UrlBuilder::fromEnvVars($baseUrl, $secret); | ||
|
||
// initialize deprecated properties | ||
$this->bbbBaseUrl = $bbbBaseUrl; | ||
$this->bbbSecret = $bbbSecret; | ||
$this->hashingAlgorithm = $hashingAlgorithm; | ||
$this->bbbBaseUrl = $urlBuilder->getBaseUrl(); | ||
$this->bbbSecret = $urlBuilder->getSecret(); | ||
$this->hashingAlgorithm = $urlBuilder->getHashingAlgorithm(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need these getters if we want to fill the deprecated properties. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, maybe we can leave the properties uninitialized? Any code that currently uses these properties would do so by extending BigBlueButton. |
||
|
||
$this->urlBuilder = new UrlBuilder($bbbSecret, $bbbBaseUrl, $hashingAlgorithm); | ||
$this->urlBuilder = $urlBuilder; | ||
$this->curlOpts = $opts['curl'] ?? []; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,42 @@ public function __construct(string $secret, string $baseUrl, HashingAlgorithm $h | |
$this->setHashingAlgorithm($hashingAlgorithm); | ||
} | ||
|
||
/** | ||
* Creates a new instance from environment variables. | ||
* | ||
* The optional parameters allow to specify some of the values, while the | ||
* remaining values fall back to environment variables or to a default | ||
* value. | ||
* | ||
* This method only exists to BC-support creating a BigBlueButton class | ||
* without injecting the UrlBuilder instance. | ||
* | ||
* @internal | ||
*/ | ||
public static function fromEnvVars( | ||
?string $secret = null, | ||
?string $baseUrl = null, | ||
?HashingAlgorithm $hashingAlgorithm = null, | ||
): static { | ||
$secret ??= getenv('BBB_SECRET') ?: getenv('BBB_SECURITY_SALT'); | ||
if (false === $secret) { | ||
throw new \RuntimeException("No BBB-Secret (or BBB-Salt) found! Please provide it either in constructor (2nd argument) or by environment variable 'BBB_SECRET' (or 'BBB_SECURITY_SALT')!"); | ||
} | ||
|
||
$baseUrl ??= getenv('BBB_SERVER_BASE_URL'); | ||
if (false === $baseUrl) { | ||
throw new \RuntimeException('No BBB-Server-Url found! Please provide it either in constructor ' | ||
. "(1st argument) or by environment variable 'BBB_SERVER_BASE_URL'!"); | ||
} | ||
|
||
$hashingAlgorithm ??= HashingAlgorithm::SHA_256; | ||
|
||
// Extending classes need to override this method, if they change the | ||
// constructor signature. | ||
// @phpstan-ignore new.static | ||
return new static($secret, $baseUrl, $hashingAlgorithm); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to decide whether the env vars should be still supported in the next major version. |
||
|
||
// Getters & Setters | ||
public function setSecret(string $secret): self | ||
{ | ||
|
@@ -62,6 +98,19 @@ public function setSecret(string $secret): self | |
return $this; | ||
} | ||
|
||
/** | ||
* Gets the secret. | ||
* | ||
* This method only exists to support a deprecated property in the | ||
* BigBlueButton class. | ||
* | ||
* @internal | ||
*/ | ||
public function getSecret(): string | ||
{ | ||
return $this->secret; | ||
} | ||
|
||
public function setBaseUrl(string $baseUrl): self | ||
{ | ||
// add tailing dir-separator if missing | ||
|
@@ -74,6 +123,19 @@ public function setBaseUrl(string $baseUrl): self | |
return $this; | ||
} | ||
|
||
/** | ||
* Gets the base url. | ||
* | ||
* This method only exists to support a deprecated property in the | ||
* BigBlueButton class. | ||
* | ||
* @internal | ||
*/ | ||
public function getBaseUrl(): string | ||
{ | ||
return $this->baseUrl; | ||
} | ||
|
||
public function setHashingAlgorithm(HashingAlgorithm $hashingAlgorithm): self | ||
{ | ||
$this->hashingAlgorithm = $hashingAlgorithm; | ||
|
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 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.