-
Notifications
You must be signed in to change notification settings - Fork 19
Add getEnvVariable utility method to fix Laravel config caching issue #293
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
Changes from 4 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 |
|---|---|---|
|
|
@@ -43,8 +43,9 @@ public static function getApiKey() | |
| return self::$apiKey; | ||
| } | ||
|
|
||
| if (getenv("WORKOS_API_KEY")) { | ||
| self::$apiKey = getenv("WORKOS_API_KEY"); | ||
| $envValue = self::getEnvVariable("WORKOS_API_KEY"); | ||
| if ($envValue) { | ||
| self::$apiKey = $envValue; | ||
| return self::$apiKey; | ||
| } | ||
|
|
||
|
|
@@ -71,8 +72,9 @@ public static function getClientId() | |
| return self::$clientId; | ||
| } | ||
|
|
||
| if (getenv("WORKOS_CLIENT_ID")) { | ||
| self::$clientId = getenv("WORKOS_CLIENT_ID"); | ||
| $envValue = self::getEnvVariable("WORKOS_CLIENT_ID"); | ||
| if ($envValue) { | ||
| self::$clientId = $envValue; | ||
| return self::$clientId; | ||
| } | ||
|
|
||
|
|
@@ -135,4 +137,28 @@ public static function getVersion() | |
| { | ||
| return self::$version; | ||
| } | ||
|
|
||
| /** | ||
| * Get environment variable with fallback to cached config sources | ||
| * | ||
| * @param string $key Environment variable name | ||
| * @return string|false The environment variable value or false if not found | ||
|
Contributor
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. style: Return type hint could be more specific: '@return string|false The value or false. Empty string is never returned.' |
||
| */ | ||
| private static function getEnvVariable($key) | ||
| { | ||
| $value = getenv($key); | ||
| if ($value !== false && $value !== '') { | ||
| return $value; | ||
| } | ||
|
|
||
| if (isset($_ENV[$key]) && $_ENV[$key] !== '') { | ||
| return $_ENV[$key]; | ||
| } | ||
|
|
||
| if (isset($_SERVER[$key]) && $_SERVER[$key] !== '') { | ||
| return $_SERVER[$key]; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,59 @@ | ||||||||||||||||||
| <?php | ||||||||||||||||||
|
|
||||||||||||||||||
| namespace WorkOS; | ||||||||||||||||||
|
|
||||||||||||||||||
| use PHPUnit\Framework\TestCase; | ||||||||||||||||||
|
|
||||||||||||||||||
| class WorkOSTest extends TestCase | ||||||||||||||||||
| { | ||||||||||||||||||
| protected function setUp(): void | ||||||||||||||||||
| { | ||||||||||||||||||
| WorkOS::setApiKey(null); | ||||||||||||||||||
| WorkOS::setClientId(null); | ||||||||||||||||||
|
|
||||||||||||||||||
| putenv("WORKOS_API_KEY="); | ||||||||||||||||||
| putenv("WORKOS_CLIENT_ID="); | ||||||||||||||||||
|
|
||||||||||||||||||
| unset($_ENV['WORKOS_API_KEY']); | ||||||||||||||||||
| unset($_ENV['WORKOS_CLIENT_ID']); | ||||||||||||||||||
| unset($_SERVER['WORKOS_API_KEY']); | ||||||||||||||||||
| unset($_SERVER['WORKOS_CLIENT_ID']); | ||||||||||||||||||
|
Comment on lines
+14
to
+20
Contributor
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. style: Redundant cleanup of environment variables across different sources. Consider extracting this to a reusable helper method to avoid duplication with tearDown
Suggested change
|
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| protected function tearDown(): void | ||||||||||||||||||
| { | ||||||||||||||||||
| WorkOS::setApiKey(null); | ||||||||||||||||||
| WorkOS::setClientId(null); | ||||||||||||||||||
|
|
||||||||||||||||||
| putenv("WORKOS_API_KEY="); | ||||||||||||||||||
| putenv("WORKOS_CLIENT_ID="); | ||||||||||||||||||
|
|
||||||||||||||||||
| unset($_ENV['WORKOS_API_KEY']); | ||||||||||||||||||
| unset($_ENV['WORKOS_CLIENT_ID']); | ||||||||||||||||||
| unset($_SERVER['WORKOS_API_KEY']); | ||||||||||||||||||
| unset($_SERVER['WORKOS_CLIENT_ID']); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| public function testGetApiKeyFromEnvSuperglobal() | ||||||||||||||||||
| { | ||||||||||||||||||
| $_ENV['WORKOS_API_KEY'] = "pk_test_env_superglobal"; | ||||||||||||||||||
|
|
||||||||||||||||||
| $this->assertEquals("pk_test_env_superglobal", WorkOS::getApiKey()); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| public function testGetClientIdFromEnvSuperglobal() | ||||||||||||||||||
| { | ||||||||||||||||||
| $_ENV['WORKOS_CLIENT_ID'] = "client_test_env_superglobal"; | ||||||||||||||||||
|
|
||||||||||||||||||
| $this->assertEquals("client_test_env_superglobal", WorkOS::getClientId()); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| public function testLaravelConfigCachingScenario() | ||||||||||||||||||
| { | ||||||||||||||||||
| $_ENV['WORKOS_API_KEY'] = "pk_test_laravel_cached"; | ||||||||||||||||||
| $_ENV['WORKOS_CLIENT_ID'] = "client_test_laravel_cached"; | ||||||||||||||||||
|
|
||||||||||||||||||
| $this->assertEquals("pk_test_laravel_cached", WorkOS::getApiKey()); | ||||||||||||||||||
| $this->assertEquals("client_test_laravel_cached", WorkOS::getClientId()); | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+51
to
+58
Contributor
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. style: Test could be split into two separate test cases for API key and client ID to follow single responsibility principle |
||||||||||||||||||
| } | ||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.