refactor!: Context renaming #251
Conversation
Changed 'r' to 'response' and 'c' to 'callback' throughout context.dart for better code clarity and consistency.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #251 +/- ##
=======================================
Coverage 91.41% 91.41%
=======================================
Files 89 89
Lines 3458 3459 +1
Branches 1763 1763
=======================================
+ Hits 3161 3162 +1
Misses 297 297 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughSystematic refactoring renaming Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
nielsenko
force-pushed
the
context-rename
branch
2 times, most recently
from
065f443 to
c6271fc
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
lib/src/router/lookup_result.dart (1)
5-12: Typed lookup outcomes are clear and well‑documented.The sealed hierarchy and RouterMatch integration look solid. Consider adding a small helper to avoid explicit casts at call sites.
Example ergonomic helper:
extension LookupResultWhen<T> on LookupResult<T> { R when<R>({ required R Function(RouterMatch<T>) match, required R Function(PathMiss<T>) pathMiss, required R Function(MethodMiss<T>) methodMiss, }) { final self = this; if (self is RouterMatch<T>) return match(self); if (self is PathMiss<T>) return pathMiss(self); return methodMiss(self as MethodMiss<T>); } }Also applies to: 16-22, 24-33, 34-43, 45-49, 50-57
lib/src/middleware/routing_middleware.dart (1)
112-117: Extension correctly broadened to Context base type.The extension now operates on
Contextinstead ofRequestContext, making routing information accessible to all context subtypes. This is a logical improvement given the new context hierarchy.Optional: Consider renaming the extension for clarity.
The extension name
RequestContextExmay be slightly misleading now that it operates on the baseContexttype. Consider renaming toContextExorRoutingContextExin a future update to better reflect its scope, though this is a minor naming concern and may have been kept for backward compatibility.example/middleware/example.dart (2)
43-45: Extension receiver updated to Context — correct.Makes the property available on all Context variants. Consider naming the extension to avoid anonymous-extension collisions in larger apps.
-extension on Context { +extension ContextUserEx on Context { User get user => _auth[this]; }
11-25: Use a top‑level entrypoint for Isolate.spawn.Passing an async closure can fail at runtime; prefer a top‑level/static function.
- final server = await Isolate.spawn((_) async { - final app = - RelicApp() - ..use( - '/api', - AuthMiddleware().asMiddleware, - ) - ..get( - '/api/user/info', - (final ctx) => - ctx.respond(Response.ok(body: Body.fromString('${ctx.user}'))), - ); - await app.serve(); - }, null); + final server = await Isolate.spawn(_serverEntry, null); + +void _serverEntry(Object? _) async { + final app = RelicApp() + ..use('/api', AuthMiddleware().asMiddleware) + ..get( + '/api/user/info', + (final ctx) => ctx.respond(Response.ok(body: Body.fromString('${ctx.user}'))), + ); + await app.serve(); +}test/src/middleware/context_property_test.dart (1)
11-12: Update test descriptions for consistency.Several group titles still say “RequestContext” while the code uses the base “Context.” Consider updating the strings for clarity; no code changes needed.
Also applies to: 96-99, 141-145, 185-187, 215-215
lib/src/handler/handler.dart (2)
114-118: Prefer explicit parameter typing in respondWith for clarity.Adding the param type improves readability and catches accidental signature drift.
-Handler respondWith(final Responder responder) { - return (final ctx) async { +Handler respondWith(final Responder responder) { + return (final RequestContext ctx) async { return ctx.respond(await responder(ctx.request)); }; }
126-130: Ditto for hijack helper.Make the parameter type explicit.
-HijackHandler hijack(final HijackCallback callback) { - return (final ctx) { +HijackHandler hijack(final HijackCallback callback) { + return (final HijackableContext ctx) { return ctx.hijack(callback); }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
example/basic/body_example.dart(9 hunks)example/context/context_example.dart(7 hunks)example/context/context_property_example.dart(1 hunks)example/example.dart(1 hunks)example/middleware/auth_example.dart(2 hunks)example/middleware/cors_example.dart(2 hunks)example/middleware/example.dart(1 hunks)example/middleware/middleware_example.dart(4 hunks)example/middleware/pipeline_example.dart(3 hunks)lib/src/adapter/context.dart(5 hunks)lib/src/handler/handler.dart(8 hunks)lib/src/io/static/static_handler.dart(12 hunks)lib/src/middleware/context_property.dart(4 hunks)lib/src/middleware/middleware.dart(2 hunks)lib/src/middleware/middleware_logger.dart(1 hunks)lib/src/middleware/routing_middleware.dart(1 hunks)lib/src/relic_server.dart(1 hunks)lib/src/router/lookup_result.dart(1 hunks)lib/src/router/router.dart(1 hunks)lib/src/router/router_handler_extension.dart(1 hunks)test/handler/pipeline_test.dart(1 hunks)test/hijack/relic_hijack_test.dart(1 hunks)test/middleware/routing_middleware_test.dart(8 hunks)test/router/router_inject_test.dart(1 hunks)test/src/adapter/context_test.dart(4 hunks)test/src/middleware/context_property_test.dart(2 hunks)test/util/test_util.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.dart: All Dart code must pass static analysis usingdart analyze --fatal-infoswith no issues
All Dart files must be formatted withdart format(CI enforcesdart format --set-exit-if-changed .)
Files:
example/example.dartlib/src/router/router.dartexample/context/context_property_example.dartlib/src/middleware/routing_middleware.dartexample/middleware/example.dartlib/src/router/router_handler_extension.dartexample/middleware/middleware_example.dartexample/middleware/cors_example.dartexample/middleware/auth_example.darttest/src/middleware/context_property_test.dartexample/middleware/pipeline_example.darttest/middleware/routing_middleware_test.darttest/hijack/relic_hijack_test.darttest/src/adapter/context_test.darttest/router/router_inject_test.darttest/util/test_util.dartlib/src/middleware/middleware.dartlib/src/middleware/middleware_logger.dartlib/src/router/lookup_result.darttest/handler/pipeline_test.dartlib/src/handler/handler.dartexample/basic/body_example.dartlib/src/io/static/static_handler.dartexample/context/context_example.dartlib/src/middleware/context_property.dartlib/src/adapter/context.dartlib/src/relic_server.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
lib/**/*.dart: Use Uint8List for request/response bodies for performance; avoid List for body payloads
Use type-safe HTTP header parsing and validation when accessing headers
Use router with trie-based matching and symbol-based path parameters (e.g.,#name,#age) for routing
Ensure WebSocket handling includes proper lifecycle management (e.g., ping/pong for connection health)
Files:
lib/src/router/router.dartlib/src/middleware/routing_middleware.dartlib/src/router/router_handler_extension.dartlib/src/middleware/middleware.dartlib/src/middleware/middleware_logger.dartlib/src/router/lookup_result.dartlib/src/handler/handler.dartlib/src/io/static/static_handler.dartlib/src/middleware/context_property.dartlib/src/adapter/context.dartlib/src/relic_server.dart
test/**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
test/**/*.dart: Tests should follow the Given-When-Then pattern in descriptions (flexible structuring allowed)
Use Arrange-Act-Assert pattern within test bodies
Provide clear, descriptive test titles; prefer single responsibility per test unless related assertions improve clarity
Place tests in thetest/directory mirroring thelib/structure
Files:
test/src/middleware/context_property_test.darttest/middleware/routing_middleware_test.darttest/hijack/relic_hijack_test.darttest/src/adapter/context_test.darttest/router/router_inject_test.darttest/util/test_util.darttest/handler/pipeline_test.dart
🧠 Learnings (2)
📚 Learning: 2025-05-23T04:15:01.420Z
Learnt from: nielsenko
PR: serverpod/relic#76
File: lib/src/handler/handler.dart:20-20
Timestamp: 2025-05-23T04:15:01.420Z
Learning: In the relic codebase, ResponseContext is a subtype of HandledContext, making functions with signature ResponseContext Function(RequestContext) assignable to the Handler typedef (FutureOr<HandledContext> Function(NewContext ctx)) due to covariant return types and contravariant parameter types in Dart's type system.
Applied to files:
example/example.dartexample/context/context_property_example.dartexample/middleware/middleware_example.dartexample/middleware/cors_example.dartexample/middleware/auth_example.dartexample/middleware/pipeline_example.darttest/util/test_util.darttest/handler/pipeline_test.dartlib/src/handler/handler.dartexample/basic/body_example.dartexample/context/context_example.dartlib/src/adapter/context.dart
📚 Learning: 2025-10-09T16:21:09.310Z
Learnt from: CR
PR: serverpod/relic#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T16:21:09.310Z
Learning: Applies to lib/**/*.dart : Use router with trie-based matching and symbol-based path parameters (e.g., `#name`, `#age`) for routing
Applied to files:
lib/src/router/lookup_result.dart
🔇 Additional comments (25)
lib/src/middleware/middleware_logger.dart (1)
25-27: Rename alignment in logger switch is correct.Matches the new HijackedContext and ConnectionContext types; behavior unchanged. LGTM.
test/handler/pipeline_test.dart (1)
31-35: Signature update to RequestContext is correct.Test remains valid; aligns with Handler using RequestContext. LGTM.
lib/src/router/router.dart (1)
320-323: Doc rename verification passed - no issues found.The documentation examples correctly reference
RequestContextandResponseContext, which are properly defined in the codebase. The legacy name search found no stragglers. The type signatures in the doc examples are consistent with the actual Handler typedef.example/example.dart (1)
25-25: LGTM! Clean type rename.The parameter type update from
NewContexttoRequestContextis consistent with the repository-wide context refactoring. Function behavior is preserved.lib/src/relic_server.dart (1)
128-132: LGTM! Consistent context type updates.The switch cases correctly use the renamed context types (
HijackedContextandConnectionContext). The adapter method calls maintain the same behavior, passing the appropriate callbacks as before.lib/src/middleware/middleware.dart (1)
26-26: LGTM! Documentation updated to reflect new context naming.The documentation examples and comments have been correctly updated to use
RequestContext, maintaining clarity and consistency with the refactored API.Also applies to: 98-98
test/middleware/routing_middleware_test.dart (1)
37-37: LGTM! Test handler signatures updated consistently.All test handlers have been updated to accept
RequestContextinstead ofNewContext, maintaining test coverage and behavior while aligning with the refactored type hierarchy.Also applies to: 62-62, 88-88, 129-130, 177-178, 256-257, 303-304, 348-350
lib/src/router/router_handler_extension.dart (1)
27-27: LGTM! Router handler extension updated correctly.The
callmethod signature now acceptsRequestContext, maintaining consistency with the refactored context types while preserving all pipeline behavior.example/context/context_property_example.dart (1)
21-21: LGTM! Handler signature updated consistently.The parameter type change to
RequestContextaligns with the repository-wide refactoring, with no changes to the handler's logic or behavior.example/middleware/cors_example.dart (1)
10-10: LGTM! CORS example updated with new context types.Both the middleware inner handler and the API handler signatures have been updated to use
RequestContext, maintaining all CORS functionality and request handling behavior.Also applies to: 43-43
test/router/router_inject_test.dart (1)
142-145: RequestContext rename applied correctly.Signature now matches HandlerObject.call and Handler typedef; return remains ResponseContext (a HandledContext). Looks good. Based on learnings.
example/middleware/pipeline_example.dart (1)
8-9: Consistent RequestContext migration across middleware and handlers.Types align with Handler(RequestContext) and ResponseContext flow. No behavior changes.
Also applies to: 26-26, 44-48
test/src/middleware/context_property_test.dart (1)
5-8: Factory now returns Context — good alignment.Using Context for _createContextInstance and locals keeps tests compatible across RequestContext/ResponseContext transitions.
test/util/test_util.dart (1)
15-15: Typedef and handler params migrated to RequestContext — OK.Types align with Handler(RequestContext); tests remain readable and unchanged in behavior.
Also applies to: 26-37, 42-44
example/middleware/auth_example.dart (1)
8-9: RequestContext migration applied cleanly.Middleware and handlers now match the updated public API.
Also applies to: 25-27, 30-32
test/src/adapter/context_test.dart (1)
7-13: Tests reflect renamed context types and transitions.Assertions cover RequestContext → ResponseContext/HijackedContext/ConnectionContext and token preservation. Looks solid.
Also applies to: 26-28, 57-63, 65-71, 76-79, 97-101
lib/src/handler/handler.dart (2)
62-62: Core Handler typedef switched to RequestContext — good.Public API shift is consistent with downstream examples/tests; variance with ResponseContext <: HandledContext remains sound. Based on learnings.
145-149: No changes needed—RelicRouter type aliasing is correctly implemented.RelicRouter is defined as
typedef RelicRouter = Router<Handler>;, making it semantically identical toRouter<Handler>at compile time. HandlerObject.injectIn correctly usesfinal RelicRouter router, and the type alias ensures that@overridechecks pass underdart analyze --fatal-infoswithout any compatibility issues. The code is already correct.example/middleware/middleware_example.dart (1)
10-10: LGTM! Context type updates are consistent.All middleware and handler signatures correctly updated from
NewContexttoRequestContext. The changes are pure renames with no behavioral modifications.Also applies to: 30-30, 46-46, 61-61, 67-67
example/context/context_example.dart (1)
9-12: LGTM! Comprehensive context renaming applied correctly.All documentation, HTML content, and handler signatures consistently updated to reflect the new context naming:
RequestContextfor initial request stateConnectionContextfor WebSocket connectionsHijackedContextfor raw connection controlAlso applies to: 27-28, 36-118
lib/src/middleware/context_property.dart (1)
3-7: LGTM! API correctly broadened to accept base Context type.The update from
RequestContexttoContextis appropriate sinceContextis now the sealed base class containing thetokenproperty. This allowsContextPropertyto work with all context types (RequestContext, ResponseContext, HijackedContext, ConnectionContext) while maintaining type safety.Also applies to: 45-82
example/basic/body_example.dart (2)
53-53: LGTM! Lambda parameter simplification aligns with PR objectives.Removal of unused
finalmodifiers in lambda parameters is a style cleanup explicitly mentioned in the PR description.Also applies to: 62-62
83-189: LGTM! All handler signatures consistently updated.All handler entry points correctly updated from
NewContexttoRequestContextwith no behavioral changes.lib/src/io/static/static_handler.dart (1)
35-35: LGTM! Static handler signatures consistently updated.The
CacheControlFactorytypedef and all internal method signatures correctly updated fromNewContexttoRequestContext. No behavioral changes introduced.Also applies to: 129-428
lib/src/adapter/context.dart (1)
1-200: Refactor verification complete — all context names properly updated.Verification results confirm:
- ✅ No references to old context names (NewContext, HijackContext, ConnectContext)
- ✅ New context names consistently used throughout: RequestContext in tests/examples/handlers, HijackedContext in relic_server.dart/middleware_logger.dart/handler.dart, ConnectionContext similarly deployed
- ✅ Public API correctly exports all context types via lib/relic.dart with RequestInternal appropriately hidden
- ✅ Pattern matching on sealed contexts in lib/src/relic_server.dart and lib/src/middleware/middleware_logger.dart demonstrates correct sealed class exhaustiveness checking
- ✅ Tests verify state transitions work properly (RequestContext → ResponseContext/HijackedContext/ConnectionContext)
- ✅ Examples demonstrate correct usage patterns
The context hierarchy refactor is complete and production-ready.
nielsenko
force-pushed
the
context-rename
branch
from
c6271fc to
6390236
Compare
…ond & ResponseContext)
… & ResponseContext)
nielsenko
force-pushed
the
context-rename
branch
from
6390236 to
4e463ab
Compare
SandPod
left a comment
SandPod
left a comment
There was a problem hiding this comment.
Question regarding version support in Serverpod.
2 participants
Description
Renames core context types for improved clarity and consistency:
NewContext→RequestContextConnectContext→ConnectionContextHijackContext→HijackedContextAdditionally:
RequestContextrenamed toContextRequestContextExrenamed to match new namingfinalparameter modifiers in lambda expressionsRelated Issues
Pre-Launch Checklist
///), ensuring consistency with existing project documentation. BUT NOT DOCUSAURUS SITEBreaking Changes
Type Renames
NewContextRequestContextConnectContextConnectionContextHijackContextHijackedContextRequestContext(sealed base)Context(sealed base)All handler signatures must be updated to use the new type names.
Additional Notes
This is a pure rename refactor with no behavioral changes. The rename improves semantic clarity—
RequestContextbetter describes the initial request state, whileContextserves as the appropriate base class name.Docusaurus site is not updated as to avoid excessive conflicts with inflight doc PRs.
Summary by CodeRabbit
New Features
Refactor