Skip to content

Commit 7ea011c

Browse files
roxblnfkroxblnfk
committed
refactor(ConfigClient): improve profile loading logic and update method signatures
1 parent 915f190 commit 7ea011c

File tree

2 files changed

+94
-38
lines changed

2 files changed

+94
-38
lines changed

‎src/Common/EnvConfig/ConfigClient.php‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
use Temporal\Common\EnvConfig\Exception\ProfileNotFoundException;
1515

1616
/**
17-
* Client configuration container managing multiple named profiles.
17+
* Client configuration loaded from TOML and environment variables.
1818
*
1919
* This class provides methods to load configuration from TOML files and environment variables,
2020
* following the Temporal external client configuration specification.
@@ -41,7 +41,7 @@ private function __construct(
4141
) {}
4242

4343
/**
44-
* Load client configuration with optional file and environment variable merging.
44+
* Load a single client profile from given sources, applying env overrides.
4545
*
4646
* This is the primary method for loading configuration with full control over sources.
4747
*
@@ -64,11 +64,12 @@ public static function load(
6464
?string $profileName = null,
6565
?string $configFile = null,
6666
array $env = [],
67-
): self {
67+
): ConfigProfile {
6868
$env = $env ?: \getenv();
6969

7070
// Load environment config first to get profile name if not specified
7171
$envConfig = ConfigEnv::fromEnv($env);
72+
$profileExpected = $profileName !== null || $envConfig->currentProfile !== null;
7273
$profileName ??= $envConfig->currentProfile ?? 'default';
7374
$profileNameLower = \strtolower($profileName);
7475

@@ -80,12 +81,9 @@ public static function load(
8081
}
8182

8283
// Load from file if it exists
83-
$profile = null;
84-
$profiles = [];
85-
if ($configFile !== null) {
86-
$profiles = self::loadFromToml($configFile)->profiles;
87-
$profile = $profiles[$profileNameLower] ?? null;
88-
}
84+
$profile = $configFile === null
85+
? null
86+
: self::loadFromToml($configFile)->profiles[$profileNameLower] ?? null;
8987

9088
// Merge with environment overrides or use env profile
9189
if ($profile !== null) {
@@ -95,30 +93,26 @@ public static function load(
9593
$profile = $envConfig->profile;
9694
}
9795

98-
// If still no profile found, throw
99-
$profile === null and throw new ProfileNotFoundException($profileName);
96+
if ($profile !== null) {
97+
return $profile;
98+
}
10099

101-
// Store profile with lowercase key
102-
$profiles[$profileNameLower] = $profile;
100+
$profileExpected and throw new ProfileNotFoundException($profileName);
103101

104-
return new self($profiles);
102+
// Returns empty profile if default doesn't exist and wasn't explicitly requested
103+
return new ConfigProfile(null, null, null);
105104
}
106105

107106
/**
108-
* Load client configuration from environment variables only.
107+
* Load a single profile directly from environment variables.
109108
*
110109
* Uses TEMPORAL_* environment variables to construct configuration.
111110
*
112111
* @param array $env Environment variables array.
113112
*/
114-
public static function loadFromEnv(array $env = []): self
113+
public static function loadFromEnv(array $env = []): ConfigProfile
115114
{
116-
$env = $env ?: \getenv();
117-
118-
$envConfig = ConfigEnv::fromEnv($env);
119-
$profileName = $envConfig->currentProfile ?? 'default';
120-
121-
return new self([$profileName => $envConfig->profile]);
115+
return ConfigEnv::fromEnv($env ?: \getenv())->profile;
122116
}
123117

124118
/**

‎tests/Unit/Common/EnvConfig/ConfigClientTest.php‎

Lines changed: 78 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,10 @@ public function testLoadFromEnvWithSystemEnvProvider(): void
8484
$this->env['TEMPORAL_API_KEY'] = 'test-key';
8585

8686
// Act
87-
$config = ConfigClient::loadFromEnv($this->env);
87+
$profile = ConfigClient::loadFromEnv($this->env);
8888

8989
// Assert
90-
self::assertInstanceOf(ConfigClient::class, $config);
91-
$profile = $config->getProfile('default');
90+
self::assertInstanceOf(ConfigProfile::class, $profile);
9291
self::assertSame('localhost:7233', $profile->address);
9392
self::assertSame('test-namespace', $profile->namespace);
9493
self::assertSame('test-key', $profile->apiKey);
@@ -97,11 +96,10 @@ public function testLoadFromEnvWithSystemEnvProvider(): void
9796
public function testLoadFromEnvWithEmptyEnvironment(): void
9897
{
9998
// Act
100-
$config = ConfigClient::loadFromEnv($this->env);
99+
$profile = ConfigClient::loadFromEnv($this->env);
101100

102101
// Assert
103-
self::assertInstanceOf(ConfigClient::class, $config);
104-
$profile = $config->getProfile('default');
102+
self::assertInstanceOf(ConfigProfile::class, $profile);
105103
self::assertNull($profile->address);
106104
self::assertNull($profile->namespace);
107105
self::assertNull($profile->apiKey);
@@ -117,13 +115,13 @@ public function testLoadFromFileOnly(): void
117115
TOML;
118116

119117
// Act
120-
$config = ConfigClient::load(
118+
$profile = ConfigClient::load(
121119
profileName: 'default',
122120
configFile: $toml,
123121
);
124122

125123
// Assert
126-
$profile = $config->getProfile('default');
124+
self::assertInstanceOf(ConfigProfile::class, $profile);
127125
self::assertSame('127.0.0.1:7233', $profile->address);
128126
self::assertSame('default', $profile->namespace);
129127
}
@@ -141,14 +139,14 @@ public function testLoadWithEnvOverrides(): void
141139
$this->env['TEMPORAL_NAMESPACE'] = 'override-namespace';
142140

143141
// Act
144-
$config = ConfigClient::load(
142+
$profile = ConfigClient::load(
145143
profileName: 'default',
146144
configFile: $toml,
147145
env: $this->env,
148146
);
149147

150148
// Assert
151-
$profile = $config->getProfile('default');
149+
self::assertInstanceOf(ConfigProfile::class, $profile);
152150
self::assertSame('override.temporal.io:7233', $profile->address);
153151
self::assertSame('override-namespace', $profile->namespace);
154152
}
@@ -167,14 +165,14 @@ public function testLoadUsesTemporalProfileEnvVar(): void
167165
$this->env['TEMPORAL_PROFILE'] = 'production';
168166

169167
// Act
170-
$config = ConfigClient::load(
168+
$profile = ConfigClient::load(
171169
profileName: null,
172170
configFile: $toml,
173171
env: $this->env,
174172
);
175173

176174
// Assert
177-
$profile = $config->getProfile('production');
175+
self::assertInstanceOf(ConfigProfile::class, $profile);
178176
self::assertSame('prod.temporal.io:7233', $profile->address);
179177
}
180178

@@ -187,14 +185,14 @@ public function testLoadDefaultsToDefaultProfile(): void
187185
TOML;
188186

189187
// Act
190-
$config = ConfigClient::load(
188+
$profile = ConfigClient::load(
191189
profileName: null,
192190
configFile: $toml,
193191
env: $this->env,
194192
);
195193

196194
// Assert
197-
$profile = $config->getProfile('default');
195+
self::assertInstanceOf(ConfigProfile::class, $profile);
198196
self::assertSame('127.0.0.1:7233', $profile->address);
199197
}
200198

@@ -217,21 +215,85 @@ public function testLoadThrowsExceptionWhenProfileNotFound(): void
217215
);
218216
}
219217

218+
public function testLoadReturnsEmptyProfileWhenDefaultNotFoundAndNotExplicitlyRequested(): void
219+
{
220+
// Arrange
221+
$toml = <<<'TOML'
222+
[profile.production]
223+
address = "prod.temporal.io:7233"
224+
TOML;
225+
226+
// Act - No profile name specified, no TEMPORAL_PROFILE env var, and no 'default' profile in TOML
227+
$profile = ConfigClient::load(
228+
profileName: null,
229+
configFile: $toml,
230+
env: $this->env,
231+
);
232+
233+
// Assert - Returns empty profile instead of throwing exception (matches Rust behavior)
234+
self::assertInstanceOf(ConfigProfile::class, $profile);
235+
self::assertNull($profile->address);
236+
self::assertNull($profile->namespace);
237+
self::assertNull($profile->apiKey);
238+
}
239+
240+
public function testLoadThrowsExceptionWhenDefaultProfileExplicitlyRequestedButNotFound(): void
241+
{
242+
// Arrange
243+
$toml = <<<'TOML'
244+
[profile.production]
245+
address = "prod.temporal.io:7233"
246+
TOML;
247+
248+
// Assert (before Act for exceptions)
249+
$this->expectException(ProfileNotFoundException::class);
250+
$this->expectExceptionMessage("Profile 'default' not found");
251+
252+
// Act - Explicitly requesting 'default' profile that doesn't exist
253+
ConfigClient::load(
254+
profileName: 'default',
255+
configFile: $toml,
256+
env: $this->env,
257+
);
258+
}
259+
260+
public function testLoadThrowsExceptionWhenProfileRequestedViaEnvVarButNotFound(): void
261+
{
262+
// Arrange
263+
$toml = <<<'TOML'
264+
[profile.default]
265+
address = "127.0.0.1:7233"
266+
TOML;
267+
268+
$this->env['TEMPORAL_PROFILE'] = 'staging';
269+
270+
// Assert (before Act for exceptions)
271+
$this->expectException(ProfileNotFoundException::class);
272+
$this->expectExceptionMessage("Profile 'staging' not found");
273+
274+
// Act - Profile requested via TEMPORAL_PROFILE env var but doesn't exist
275+
ConfigClient::load(
276+
profileName: null,
277+
configFile: $toml,
278+
env: $this->env,
279+
);
280+
}
281+
220282
public function testLoadFromEnvOnlyWhenNoFileProvided(): void
221283
{
222284
// Arrange
223285
$this->env['TEMPORAL_ADDRESS'] = 'env.temporal.io:7233';
224286
$this->env['TEMPORAL_NAMESPACE'] = 'env-namespace';
225287

226288
// Act
227-
$config = ConfigClient::load(
289+
$profile = ConfigClient::load(
228290
profileName: 'default',
229291
configFile: null,
230292
env: $this->env,
231293
);
232294

233295
// Assert
234-
$profile = $config->getProfile('default');
296+
self::assertInstanceOf(ConfigProfile::class, $profile);
235297
self::assertSame('env.temporal.io:7233', $profile->address);
236298
self::assertSame('env-namespace', $profile->namespace);
237299
}

0 commit comments

Comments
 (0)