Skip to content

Commit def2748

Browse files
committed
review cleanup
1 parent c98e127 commit def2748

File tree

6 files changed

+70
-16
lines changed

6 files changed

+70
-16
lines changed

lib/src/common/environment.dart

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,28 @@ class FirebaseEnv {
8787
///
8888
/// Uses the [PORT] environment variable, defaulting to 8080.
8989
int get port => int.tryParse(environment['PORT'] ?? '8080') ?? 8080;
90+
91+
/// The name of the Cloud Run service.
92+
///
93+
/// Uses the `K_SERVICE` environment variable.
94+
///
95+
/// See https://cloud.google.com/run/docs/container-contract#env-vars
96+
String? get kService => environment['K_SERVICE'];
97+
98+
/// The name of the target function.
99+
///
100+
/// Uses the `FUNCTION_TARGET` environment variable.
101+
///
102+
/// See https://docs.cloud.google.com/run/docs/configuring/services/environment-variables#additional_reserved_environment_variables_when_deploying_functions
103+
String? get functionTarget => environment['FUNCTION_TARGET'];
104+
105+
/// Whether the functions control API is enabled.
106+
///
107+
/// Uses the `FUNCTIONS_CONTROL_API` environment variable.
108+
///
109+
/// This is part of the contract with `firebase-tools`.
110+
bool get functionsControlApi =>
111+
environment['FUNCTIONS_CONTROL_API'] == 'true';
90112
}
91113

92114
/// Common project ID environment variables checked in order.

lib/src/firebase.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,5 +255,5 @@ abstract class FunctionsNamespace {
255255
/// Internal extension to access private members of Firebase.
256256
@internal
257257
extension FirebaseInternal on Firebase {
258-
FirebaseEnv get envInternal => _env;
258+
FirebaseEnv get $env => _env;
259259
}

lib/src/https/https_namespace.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class HttpsNamespace extends FunctionsNamespace {
100100
}
101101

102102
// Extract auth and app check tokens
103-
final skipVerification = firebase.envInternal.skipTokenVerification;
103+
final skipVerification = firebase.$env.skipTokenVerification;
104104
final tokens = await checkTokens(
105105
request,
106106
skipTokenVerification: skipVerification,
@@ -180,7 +180,7 @@ class HttpsNamespace extends FunctionsNamespace {
180180
final body = await request.json as Map<String, dynamic>?;
181181

182182
// Extract auth and app check tokens
183-
final skipVerification = firebase.envInternal.skipTokenVerification;
183+
final skipVerification = firebase.$env.skipTokenVerification;
184184
final tokens = await checkTokens(
185185
request,
186186
skipTokenVerification: skipVerification,

lib/src/identity/identity_namespace.dart

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -276,20 +276,18 @@ class IdentityNamespace extends FunctionsNamespace {
276276
/// skipTokenVerification debug feature is enabled), verification is skipped.
277277
Future<Map<String, dynamic>> _decodeAndVerifyJwt(String jwt) async {
278278
// Get project ID
279-
final projectId = firebase.envInternal.projectId;
279+
final projectId = firebase.$env.projectId;
280280

281281
// Create verifier
282282
final verifier = AuthBlockingTokenVerifier(
283283
projectId: projectId,
284284
isEmulator:
285-
firebase.envInternal.isEmulator ||
286-
firebase.envInternal.skipTokenVerification,
285+
firebase.$env.isEmulator || firebase.$env.skipTokenVerification,
287286
);
288287

289288
// Determine audience based on platform
290289
// Cloud Run uses "run.app", GCF v1 uses default
291-
final kService =
292-
firebase.envInternal.environment['K_SERVICE']; // Cloud Run service name
290+
final kService = firebase.$env.kService; // Cloud Run service name
293291
final audience = kService != null ? 'run.app' : null;
294292

295293
return verifier.verifyToken(jwt, audience: audience);

lib/src/server.dart

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ typedef FunctionsRunner = FutureOr<void> Function(Firebase firebase);
4646
/// ```
4747
Future<void> fireUp(List<String> args, FunctionsRunner runner) async {
4848
final firebase = Firebase();
49-
final env = firebase.envInternal;
49+
final env = firebase.$env;
5050
final projectId = env.projectId;
5151

5252
await runZoned(zoneValues: {projectIdZoneKey: projectId}, () async {
@@ -60,7 +60,7 @@ Future<void> fireUp(List<String> args, FunctionsRunner runner) async {
6060
final traceHeader = request.headers[cloudTraceContextHeader];
6161
String? traceId;
6262
if (traceHeader != null) {
63-
traceId = 'projects/$projectId/traces/${traceHeader.split('/')[0]}';
63+
traceId = traceHeader.split('/')[0];
6464
}
6565

6666
if (traceId == null) {
@@ -127,15 +127,14 @@ FutureOr<Response> _routeRequest(
127127
return _handleQuitQuitQuit(request);
128128
}
129129

130-
if (requestPath == '__/functions.yaml' &&
131-
env.environment['FUNCTIONS_CONTROL_API'] == 'true') {
130+
if (requestPath == '__/functions.yaml' && env.functionsControlApi) {
132131
// Manifest endpoint for function discovery
133132
return _handleFunctionsManifest(request, firebase);
134133
}
135134

136135
// FUNCTION_TARGET mode (production): Serve only the specified function
137136
// This matches Node.js behavior where each Cloud Run service runs one function
138-
final functionTarget = env.environment['FUNCTION_TARGET'];
137+
final functionTarget = env.functionTarget;
139138
if (functionTarget != null && functionTarget.isNotEmpty) {
140139
return _routeToTargetFunction(request, firebase, env, functionTarget);
141140
}

test/unit/logger_test.dart

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ void main() {
293293

294294
group('trace context', () {
295295
test(
296-
'should add trace header when traceId is in Zone',
296+
'should add trace header when both projectId and traceId are in Zone',
297297
() {
298298
runZoned(
299299
() {
@@ -306,12 +306,47 @@ void main() {
306306
});
307307
},
308308
zoneValues: {
309+
projectIdZoneKey: 'test-project',
309310
traceIdZoneKey: 'abc123',
310-
// Simulate GCLOUD_PROJECT env var via a zone-aware override
311311
},
312312
);
313313
},
314-
skip: 'Requires GCLOUD_PROJECT env var to be set',
314+
);
315+
316+
test(
317+
'should not add trace header when projectId is missing in Zone',
318+
() {
319+
runZoned(
320+
() {
321+
testLogger.write({'severity': 'INFO', 'message': 'traced'});
322+
expect(parseStdout(), {
323+
'severity': 'INFO',
324+
'message': 'traced',
325+
});
326+
},
327+
zoneValues: {
328+
traceIdZoneKey: 'abc123',
329+
},
330+
);
331+
},
332+
);
333+
334+
test(
335+
'should not add trace header when traceId is missing in Zone',
336+
() {
337+
runZoned(
338+
() {
339+
testLogger.write({'severity': 'INFO', 'message': 'traced'});
340+
expect(parseStdout(), {
341+
'severity': 'INFO',
342+
'message': 'traced',
343+
});
344+
},
345+
zoneValues: {
346+
projectIdZoneKey: 'test-project',
347+
},
348+
);
349+
},
315350
);
316351
});
317352
});

0 commit comments

Comments
 (0)