-
-
Notifications
You must be signed in to change notification settings - Fork 291
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
Prepare fastify integration #2922
Conversation
WalkthroughThe changes in this pull request involve several modifications across multiple classes, primarily focusing on method signatures and enhancing asynchronous handling. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PlatformExpress
participant PlatformBuilder
Client->>PlatformExpress: Request
PlatformExpress->>PlatformBuilder: useContext()
PlatformBuilder->>PlatformBuilder: runLifecycle()
PlatformBuilder-->>PlatformExpress: Context Established
PlatformExpress-->>Client: Response
sequenceDiagram
participant Client
participant PlatformKoa
Client->>PlatformKoa: Request
PlatformKoa->>PlatformKoa: useContext()
PlatformKoa-->>Client: Response
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (7)
packages/platform/platform-express/src/components/PlatformExpress.ts
(1 hunks)packages/platform/platform-http/src/common/builder/PlatformBuilder.ts
(1 hunks)packages/platform/platform-http/src/common/middlewares/PlatformMulterMiddleware.ts
(1 hunks)packages/platform/platform-http/src/common/services/PlatformAdapter.ts
(1 hunks)packages/platform/platform-http/src/common/utils/createContext.ts
(1 hunks)packages/platform/platform-koa/src/components/PlatformKoa.ts
(1 hunks)packages/platform/platform-params/src/builder/PlatformParams.ts
(1 hunks)
🔇 Additional comments (8)
packages/platform/platform-params/src/builder/PlatformParams.ts (1)
67-68
: LGTM! Type-safe parameter compilation
The improved type safety with PlatformParamsScope<Context>
ensures better compile-time checks for context handling.
packages/platform/platform-http/src/common/middlewares/PlatformMulterMiddleware.ts (2)
42-48
: 🛠️ Refactor suggestion
Improve type safety and error handling for multer middleware
While the null check is a good addition, there are a few improvements that could make the code more robust:
- The type annotation for
middleware
should be more specific thanany
- The case where
multer
is undefined should throw a specific error rather than silently continuing
Consider applying these changes:
- const multer = this.app.multer(settings);
+ const multer = this.app.multer(settings);
+ if (!multer) {
+ throw new Error("Failed to initialize multer middleware");
+ }
- if (multer) {
- const middleware: any = multer.fields(this.getFields({fields}));
-
- return await middleware(ctx.getRequest(), ctx.getResponse());
- }
+ const middleware = multer.fields(this.getFields({fields}));
+ return await middleware(ctx.getRequest(), ctx.getResponse());
Let's verify the multer types available in the codebase:
42-48
: Verify error handling consistency across platform adapters
The error handling pattern here should be consistent with other platform adapters.
Let's check the error handling patterns in other adapters:
✅ Verification successful
Error handling is consistent with platform standards
The error handling implementation is appropriate and follows platform conventions:
- Multer errors are properly wrapped in a
MulterException
that extendsBadRequest
- The error handling preserves the original error details (message, code) while mapping to HTTP-appropriate responses
- The pattern matches other platform adapters which use try-catch blocks and throw typed exceptions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar error handling patterns in platform adapters
rg -t ts "throw new \w+Exception|catch \(.*\) {" packages/platform/platform-*/src/
Length of output: 864
Script:
#!/bin/bash
# Search for error handling patterns in platform adapters with fixed regex
rg -t ts "throw new \w+Exception" packages/platform/platform-*/src/
# Search for catch blocks separately
rg -t ts "catch \(" packages/platform/platform-*/src/
# Look specifically for multer-related error handling
rg -t ts "multer.*error|error.*multer" packages/platform/platform-*/src/
Length of output: 4465
Script:
#!/bin/bash
# Let's examine the full MulterMiddleware implementation to understand the error handling context
ast-grep --pattern 'class PlatformMulterMiddleware {
$$$
}'
# Also check for MulterException definition
rg -A 5 "class MulterException" packages/platform/platform-*/src/
Length of output: 1428
packages/platform/platform-http/src/common/builder/PlatformBuilder.ts (1)
193-193
: Properly awaiting this.adapter.useContext()
for asynchronous context initialization
The addition of await
ensures that any asynchronous operations within useContext
are completed before proceeding with the lifecycle processes, enhancing the reliability of the application's startup sequence.
packages/platform/platform-http/src/common/services/PlatformAdapter.ts (1)
45-45
: Updated useContext
method signature to support asynchronous operations
Changing the return type to Promise<void> | void
allows for asynchronous context initialization in subclasses, providing greater flexibility for different platform implementations.
packages/platform/platform-koa/src/components/PlatformKoa.ts (1)
143-143
: Adjusted useContext
method to return void
instead of this
Changing the return type to void
and removing the return
statement simplifies the method and aligns it with the updated useContext
signature in the PlatformAdapter
abstract class.
packages/platform/platform-express/src/components/PlatformExpress.ts (1)
178-178
: Adjusted useContext
method to return void
instead of this
Changing the return type to void
and removing the return
statement simplifies the method and ensures consistency with the updated useContext
signature in the PlatformAdapter
abstract class.
packages/platform/platform-http/src/common/utils/createContext.ts (1)
10-10
: Verify header access patterns across platform implementations.
Let's ensure this change aligns with how headers are accessed in different platform adapters.
✅ Verification successful
Based on the extensive code analysis, I can now provide a final response:
The header access pattern is consistent across platform implementations
The change to use req.get?.("x-request-id")
is safe and aligns with the platform's architecture:
- The base
PlatformRequest
class defines the standard header access pattern through theget()
method, which all platform adapters inherit and implement - The Koa adapter properly inherits this behavior from the base class
- The header access is abstracted through the platform's request interface, making it consistent across all implementations
- The x-request-id header is handled uniformly throughout the codebase, as evidenced by the test cases and implementations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how request headers are accessed across different platform implementations
# Look for header access patterns in platform-specific files
echo "Checking header access patterns in platform implementations..."
rg -A 2 "req\.(get|headers|header)" "packages/platform/**/Platform(Express|Koa|Fastify)*.ts"
# Look for x-request-id usage
echo "Checking x-request-id usage patterns..."
rg -A 2 "x-request-id" "packages/platform/**/*.ts"
Length of output: 530
Script:
#!/bin/bash
# Let's first find the platform adapter files
echo "Finding platform adapter files..."
fd "Platform.*\.ts$" packages/platform
# Then check header access patterns in all TypeScript files under platform
echo "Checking header access patterns..."
fd -e ts . packages/platform --exec rg -A 2 "req\.(get|headers|header)" {}
# Look for x-request-id usage in all TypeScript files under platform
echo "Checking x-request-id usage..."
fd -e ts . packages/platform --exec rg -A 2 "x-request-id" {}
Length of output: 11704
Script:
#!/bin/bash
# Let's specifically check the platform adapters' request handling implementations
echo "Checking Express adapter request handling..."
fd "PlatformExpressRequest\.ts$" packages/platform --exec cat {}
echo "Checking Koa adapter request handling..."
fd "PlatformKoaRequest\.ts$" packages/platform --exec cat {}
# Also check the base PlatformRequest implementation
echo "Checking base PlatformRequest implementation..."
fd "PlatformRequest\.ts$" packages/platform-http --exec cat {}
Length of output: 1504
Script:
#!/bin/bash
# Let's try with the correct path structure
echo "Checking Express adapter request handling..."
fd "PlatformExpressRequest\.ts$" . --exec cat {}
echo "Checking base PlatformRequest implementation..."
fd "PlatformRequest\.ts$" . --type f --exclude "*.spec.ts" --exec cat {}
# Also check for any header-related methods in platform adapters
echo "Checking header-related methods in platform code..."
rg -A 3 "headers|get\(" "packages/platform/platform-express/src/" "packages/platform/platform-koa/src/" "packages/platform/platform-http/src/"
Length of output: 67580
async getArg<Context extends DIContext = DIContext>( | ||
scope: PlatformParamsScope<Context>, | ||
pipes: PipeMethods[], | ||
param: JsonParameterStore | ||
): Promise<any> { |
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.
🧹 Nitpick (assertive)
Consider adding input validation
While the implementation is solid, consider adding validation for the input parameters to ensure they're not undefined/null before processing.
async getArg<Context extends DIContext = DIContext>(
scope: PlatformParamsScope<Context>,
pipes: PipeMethods[],
param: JsonParameterStore
): Promise<any> {
+ if (!scope || !pipes || !param) {
+ throw new Error('Invalid arguments: scope, pipes, and param are required');
+ }
+ if (!Array.isArray(pipes)) {
+ throw new Error('pipes must be an array');
+ }
try {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
async getArg<Context extends DIContext = DIContext>( | |
scope: PlatformParamsScope<Context>, | |
pipes: PipeMethods[], | |
param: JsonParameterStore | |
): Promise<any> { | |
async getArg<Context extends DIContext = DIContext>( | |
scope: PlatformParamsScope<Context>, | |
pipes: PipeMethods[], | |
param: JsonParameterStore | |
): Promise<any> { | |
if (!scope || !pipes || !param) { | |
throw new Error('Invalid arguments: scope, pipes, and param are required'); | |
} | |
if (!Array.isArray(pipes)) { | |
throw new Error('pipes must be an array'); | |
} | |
try { |
try { | ||
let value = scope; | ||
|
||
try { | ||
return await pipe.transform(value, param); | ||
} catch (er) { | ||
throw ParamValidationError.from(param, er); | ||
for await (const pipe of pipes) { | ||
value = await pipe.transform(value, param); | ||
} | ||
}, scope); | ||
|
||
return value; | ||
} catch (er) { | ||
throw ParamValidationError.from(param, er); | ||
} |
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.
🧹 Nitpick (assertive)
Consider adding debug logging
For better debugging capabilities, consider adding debug logging for pipe transformations and error cases.
try {
let value = scope;
+ const debug = require('debug')('tsed:platform:params');
for await (const pipe of pipes) {
+ debug(`Transforming value through pipe ${pipe.constructor.name}`);
value = await pipe.transform(value, param);
}
return value;
} catch (er) {
+ debug(`Parameter validation failed: ${er.message}`);
throw ParamValidationError.from(param, er);
}
Committable suggestion skipped: line range outside the PR's diff.
const multer = this.app.multer(settings); | ||
|
||
return await middleware(ctx.getRequest(), ctx.getResponse()); | ||
if (multer) { | ||
const middleware: any = multer.fields(this.getFields({fields})); | ||
|
||
return await middleware(ctx.getRequest(), ctx.getResponse()); | ||
} |
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.
🧹 Nitpick (assertive)
Consider standardizing middleware initialization pattern
As this change is part of preparing for Fastify integration, consider establishing a consistent pattern for middleware initialization across different platform adapters (Express, Koa, Fastify).
You might want to:
- Create a common interface for middleware initialization
- Define clear error boundaries for middleware initialization failures
- Document the expected behavior when middleware initialization fails
Example interface pattern:
interface PlatformMiddlewareInitializer {
initialize(settings: PlatformMulterSettings): Promise<Middleware | null>;
validateInitialization(middleware: Middleware | null): void;
}
@@ -7,7 +7,7 @@ import {PlatformRequest} from "../services/PlatformRequest.js"; | |||
import {PlatformResponse} from "../services/PlatformResponse.js"; | |||
|
|||
function defaultReqIdBuilder(req: any) { | |||
return req.get("x-request-id") || v4().split("-").join(""); | |||
return req.get?.("x-request-id") || v4().split("-").join(""); |
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.
🧹 Nitpick (assertive)
LGTM! Good preparation for Fastify integration.
The addition of optional chaining (?.
) makes the code more resilient across different HTTP frameworks, as not all frameworks implement the get
method on their request objects. This change is particularly relevant for the upcoming Fastify integration.
For even better cross-platform compatibility, consider:
- Abstracting the header retrieval logic into platform-specific adapters
- Using a more generic method like
req.headers["x-request-id"]
which is commonly available across different HTTP frameworks
…NER and PLATFORM_CONTENT_TYPE_RESOLVER
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.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (7)
packages/platform/platform-response-filter/src/constants/ANY_CONTENT_TYPE.ts
(1 hunks)packages/platform/platform-response-filter/src/index.ts
(1 hunks)packages/platform/platform-response-filter/src/services/PlatformContentTypeResolver.spec.ts
(1 hunks)packages/platform/platform-response-filter/src/services/PlatformContentTypeResolver.ts
(1 hunks)packages/platform/platform-response-filter/src/services/PlatformContentTypesContainer.ts
(1 hunks)packages/platform/platform-response-filter/src/services/PlatformResponseFilter.ts
(2 hunks)packages/platform/platform-response-filter/src/utils/getContentType.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- packages/platform/platform-response-filter/src/utils/getContentType.ts
🔇 Additional comments (14)
packages/platform/platform-response-filter/src/services/PlatformResponseFilter.ts (5)
1-2
: Dependencies imported correctly
The required dependencies for isSerializable
, BaseContext
, constant
, inject
, and injectable
are correctly imported and will be used appropriately in the class.
6-7
: New imports for content type resolution
The imports for PLATFORM_CONTENT_TYPE_RESOLVER
and PLATFORM_CONTENT_TYPES_CONTAINER
are correctly added to support the new content type resolution logic.
14-15
: Dependencies injected properly
The container
and contentTypeResolver
properties are correctly injected using the inject
function, ensuring that the class has access to the necessary services.
26-26
: Utilize contentTypeResolver for determining content type
The use of this.contentTypeResolver(data, ctx)
effectively replaces the previous method, streamlining content type determination based on the data and context.
30-30
: Resolve response filter based on content type
The this.container.resolve(bestContentType)
call appropriately retrieves the response filter corresponding to the best content type, enhancing flexibility in response handling.
packages/platform/platform-response-filter/src/constants/ANY_CONTENT_TYPE.ts (1)
1-1
: Addition of 'ANY_CONTENT_TYPE' constant
Defining the ANY_CONTENT_TYPE
constant as "*/*"
provides a useful wildcard for content type handling, improving content negotiation scenarios.
packages/platform/platform-response-filter/src/index.ts (2)
4-4
: Export 'ANY_CONTENT_TYPE' constant
The ANY_CONTENT_TYPE
constant is correctly exported, making it available for use in other modules that require content type wildcards.
9-10
: Export new services for content type handling
Exporting PlatformContentTypeResolver
and PlatformContentTypesContainer
ensures that these services are accessible throughout the application, facilitating content type resolution and response filtering.
packages/platform/platform-response-filter/src/services/PlatformContentTypesContainer.ts (2)
8-28
: Factory function effectively manages response filters
The factory
function correctly initializes and populates the containers
map with response filter tokens. This design enhances the modularity and scalability of the response filter management.
30-31
: Proper export of container type and constant
The PLATFORM_CONTENT_TYPES_CONTAINER
type and constant are appropriately defined and exported, allowing for dependency injection and type safety across the platform.
packages/platform/platform-response-filter/src/services/PlatformContentTypeResolver.ts (3)
10-27
: Content type determination logic is accurate
The getContentType
function correctly identifies the appropriate content type based on the response and endpoint metadata, with suitable fallbacks for JSON data and views.
32-46
: Content negotiation handled effectively
The resolver
function properly uses the client's Accept
header and available content types to determine the best content type, ensuring that wildcard types are appropriately filtered out.
51-54
: Correct export of resolver type and constant
The PLATFORM_CONTENT_TYPE_RESOLVER
type and constant are correctly defined and exported, enabling seamless integration and dependency injection of the content type resolver.
packages/platform/platform-response-filter/src/services/PlatformContentTypeResolver.spec.ts (1)
29-32
: LGTM! Test setup follows best practices
The test setup properly handles test isolation using beforeEach/afterEach hooks.
}, | ||
ctx | ||
); | ||
const result = contentTypeResolver(data, ctx); |
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.
Fix potential async handling issues
The contentTypeResolver calls are not awaited, which could lead to unreliable tests if the resolver is or becomes asynchronous.
Apply this pattern to all test cases:
- const result = contentTypeResolver(data, ctx);
+ const result = await contentTypeResolver(data, ctx);
Also applies to: 78-78, 95-95, 113-113
beforeEach(() => PlatformTest.create()); | ||
afterEach(() => PlatformTest.reset()); | ||
|
||
it("should return the content type (undefined)", () => { | ||
it("should return the content type (undefined)", async () => { |
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.
🧹 Nitpick (assertive)
Improve test descriptions for better clarity
The test descriptions could be more specific about the scenarios they're testing.
Consider renaming the test cases to be more descriptive:
- it("should return the content type (undefined)"
+ it("should return undefined when no content type is specified"
- it("should return the content type (object - application/json)"
+ it("should return application/json for objects with explicit content type"
- it("should return the content type (string - application/json)"
+ it("should return application/json for strings with explicit content type"
- it("should return the content type (string - text/html)"
+ it("should return text/html when explicitly specified"
- it("should return the content type (string - view)"
+ it("should return text/html when using @View decorator"
Also applies to: 47-47, 65-65, 82-82, 99-99
async function getTestFixture(contentTypes = ["application/json", "text/html"]) { | ||
const contentTypeResolver = await PlatformTest.invoke<PLATFORM_CONTENT_TYPE_RESOLVER>(PLATFORM_CONTENT_TYPE_RESOLVER, [ | ||
{ | ||
token: PLATFORM_CONTENT_TYPES_CONTAINER, | ||
use: { | ||
contentTypes | ||
} | ||
} | ||
]); | ||
|
||
const data = { | ||
test: "test" | ||
}; | ||
|
||
const ctx = PlatformTest.createRequestContext(); | ||
return { | ||
data, | ||
ctx, | ||
contentTypeResolver | ||
}; | ||
} |
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.
🧹 Nitpick (assertive)
Consider enhancing the helper function with types and documentation
The helper function is well-structured, but could benefit from TypeScript types and JSDoc documentation for better maintainability.
Consider applying these improvements:
+ /**
+ * Creates a test fixture with content type resolver and context
+ * @param contentTypes - Array of supported content types
+ * @returns Test fixture containing resolver, context and sample data
+ */
- async function getTestFixture(contentTypes = ["application/json", "text/html"]) {
+ async function getTestFixture(contentTypes: string[] = ["application/json", "text/html"]): Promise<{
+ data: Record<string, string>;
+ ctx: PlatformContext;
+ contentTypeResolver: PLATFORM_CONTENT_TYPE_RESOLVER;
+ }> {
Committable suggestion skipped: line range outside the PR's diff.
class TestController { | ||
@Get("/") | ||
@(Returns(200).ContentType("application/json")) | ||
get() {} | ||
} | ||
|
||
const ctx = PlatformTest.createRequestContext(); | ||
const {contentTypeResolver, ctx, data} = await getTestFixture(); | ||
|
||
ctx.endpoint = EndpointMetadata.get(TestController, "get"); | ||
ctx.response.getRes().statusCode = 200; | ||
|
||
vi.spyOn(ctx.response, "getContentType").mockReturnValue("application/json"); | ||
|
||
const result = getContentType( | ||
{ | ||
test: "test" | ||
}, | ||
ctx | ||
); | ||
const result = contentTypeResolver(data, ctx); | ||
|
||
expect(result).toEqual("application/json"); | ||
}); | ||
it("should return the content type (string - application/json)", () => { | ||
it("should return the content type (string - application/json)", async () => { | ||
class TestController { | ||
@Get("/") | ||
@(Returns(200).ContentType("application/json")) | ||
get() {} | ||
} | ||
|
||
const ctx = PlatformTest.createRequestContext(); | ||
const {contentTypeResolver, ctx, data} = await getTestFixture(); | ||
|
||
ctx.endpoint = EndpointMetadata.get(TestController, "get"); | ||
ctx.response.getRes().statusCode = 200; | ||
vi.spyOn(ctx.response, "getContentType").mockReturnValue("application/json"); | ||
|
||
const result = getContentType( | ||
{ | ||
test: "test" | ||
}, | ||
ctx | ||
); | ||
const result = contentTypeResolver(data, ctx); | ||
|
||
expect(result).toEqual("application/json"); | ||
}); | ||
it("should return the content type (string - text/html)", () => { | ||
it("should return the content type (string - text/html)", async () => { | ||
class TestController { | ||
@Get("/") | ||
@(Returns(200).ContentType("text/html")) | ||
get() {} | ||
} | ||
|
||
const ctx = PlatformTest.createRequestContext(); | ||
const {contentTypeResolver, ctx, data} = await getTestFixture(); | ||
|
||
ctx.endpoint = EndpointMetadata.get(TestController, "get"); | ||
ctx.response.getRes().statusCode = 200; | ||
vi.spyOn(ctx.response, "getContentType").mockReturnValue("text/html"); | ||
|
||
const result = getContentType( | ||
{ | ||
test: "test" | ||
}, | ||
ctx | ||
); | ||
const result = contentTypeResolver(data, ctx); | ||
|
||
expect(result).toEqual("text/html"); | ||
}); | ||
it("should return the content type (string - view)", () => { | ||
it("should return the content type (string - view)", async () => { | ||
class TestController { | ||
@Get("/") | ||
@Returns(200) | ||
@View("view.html") | ||
get() {} | ||
} | ||
|
||
const ctx = PlatformTest.createRequestContext(); | ||
const {contentTypeResolver, ctx, data} = await getTestFixture(); | ||
|
||
ctx.endpoint = EndpointMetadata.get(TestController, "get"); | ||
ctx.response.getRes().statusCode = 200; | ||
ctx.view = "true"; | ||
|
||
const result = getContentType( | ||
{ | ||
test: "test" | ||
}, | ||
ctx | ||
); | ||
const result = contentTypeResolver(data, ctx); | ||
|
||
expect(result).toEqual("text/html"); | ||
}); |
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.
💡 Codebase verification
Test coverage needs improvement for content type resolution
Based on the implementation file, the current test suite is missing coverage for these important scenarios:
- Content type negotiation using Accept header
- Fallback behavior when content type is ANY_CONTENT_TYPE
- Multiple content types from the operation's getContentTypeOf method
- Content type resolution when request.accepts returns multiple types
These scenarios are critical for the resolver's functionality as shown in the implementation but are not covered in the current test suite.
Example test cases to add:
- Test with Accept header containing multiple content types
- Test fallback when operation returns ANY_CONTENT_TYPE
- Test content type filtering when accepts returns "/"
🔗 Analysis chain
Verify test coverage for edge cases
While the test suite covers the main scenarios, consider adding tests for:
- Error cases (invalid content types)
- Multiple content types in Accept header
- Non-200 status codes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for existing error handling tests
rg -l "should.*error" "packages/platform/platform-response-filter/src/**/*spec.ts"
# Look for other content type related tests
rg -l "ContentType|content-type" "packages/platform/platform-response-filter/src/**/*spec.ts"
Length of output: 378
Script:
#!/bin/bash
# Let's first find the correct path using fd
fd -e spec.ts -e test.ts . packages/platform/platform-response-filter/
# Check implementation file to understand all scenarios that need testing
fd PlatformContentTypeResolver.ts . packages/platform/platform-response-filter/
Length of output: 663
Script:
#!/bin/bash
# Let's examine the implementation file to understand all scenarios
cat packages/platform/platform-response-filter/src/services/PlatformContentTypeResolver.ts
# Let's also check if there are any error tests in the test files we found
rg "should.*error|describe.*error" packages/platform/platform-response-filter/src/errors/TemplateRenderError.spec.ts packages/platform/platform-response-filter/src/services/PlatformContentTypeResolver.spec.ts packages/platform/platform-response-filter/src/services/PlatformResponseFilter.spec.ts packages/platform/platform-response-filter/src/utils/renderView.spec.ts
Length of output: 2222
🎉 This PR is included in version 8.3.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
Release Notes
New Features
ANY_CONTENT_TYPE
for handling wildcard content types.PlatformContentTypeResolver
andPlatformContentTypesContainer
for improved content type management.Bug Fixes
PlatformKoa
middleware for improved robustness.createContext
to prevent runtime errors.Refactor
useContext
method signatures acrossPlatformExpress
andPlatformKoa
.PlatformMulterMiddleware
for clarity.PlatformResponseFilter
class by removing unnecessary properties and methods.Chores