Copilot commented on code in PR #3305:
URL: https://github.com/apache/apisix-dashboard/pull/3305#discussion_r2897105076
##########
e2e/tests/services.routes.list.spec.ts:
##########
@@ -34,208 +35,217 @@ const routes: APISIXType['Route'][] = [
name: randomId('route1'),
uri: '/api/v1/test1',
methods: ['GET'],
+ labels: { test: serviceName },
},
{
name: randomId('route2'),
uri: '/api/v1/test2',
methods: ['POST'],
+ labels: { test: serviceName },
},
{
name: randomId('route3'),
uri: '/api/v1/test3',
methods: ['PUT'],
+ labels: { test: serviceName },
},
];
-// Route that uses upstream directly instead of service_id
+const upstreamRouteName = randomId('upstream-route');
const upstreamRoute: APISIXType['Route'] = {
- name: randomId('upstream-route'),
+ name: upstreamRouteName,
uri: '/api/v1/upstream-test',
methods: ['GET'],
upstream: {
nodes: [{ host: 'example.com', port: 80, weight: 100 }],
},
+ labels: { test: upstreamRouteName },
};
-// Route that belongs to another service
+const anotherServiceRouteName = randomId('another-service-route');
const anotherServiceRoute: APISIXType['Route'] = {
- name: randomId('another-service-route'),
+ name: anotherServiceRouteName,
uri: '/api/v1/another-test',
methods: ['GET'],
+ labels: { test: anotherServiceName },
};
let testServiceId: string;
let anotherServiceId: string;
const createdRoutes: string[] = [];
-test.beforeAll(async () => {
- await deleteAllRoutes(e2eReq);
- await deleteAllServices(e2eReq);
+let upstreamRouteId: string;
+let anotherServiceRouteId: string;
- // Create a test service for testing service routes
+test.beforeAll(async () => {
const serviceResponse = await postServiceReq(e2eReq, {
name: serviceName,
desc: 'Test service for route listing',
+ labels: { test: serviceName },
});
testServiceId = serviceResponse.data.value.id;
- // Create another service
const anotherServiceResponse = await postServiceReq(e2eReq, {
name: anotherServiceName,
desc: 'Another test service for route isolation testing',
+ labels: { test: anotherServiceName },
});
anotherServiceId = anotherServiceResponse.data.value.id;
- // Create test routes under the service
for (const route of routes) {
const routeResponse = await postRouteReq(e2eReq, {
- ...route,
+ ...(route as Record<string, unknown>),
service_id: testServiceId,
});
+ if (!routeResponse.data?.value) {
+ throw new Error(`Failed to create route:
${JSON.stringify(routeResponse.data)}`);
+ }
createdRoutes.push(routeResponse.data.value.id);
}
- // Create a route that uses upstream directly instead of service_id
- await postRouteReq(e2eReq, upstreamRoute);
- // Create a route under another service
- await postRouteReq(e2eReq, {
- ...anotherServiceRoute,
+ const upstreamRouteResponse = await postRouteReq(e2eReq, upstreamRoute as
unknown as Parameters<typeof postRouteReq>[1]);
+ if (!upstreamRouteResponse.data?.value) {
Review Comment:
This test uses several broad type casts (`route as Record<string, unknown>`,
`upstreamRoute as unknown as Parameters<typeof postRouteReq>[1]`) when calling
`postRouteReq`, which defeats type-checking for the request payload and can
hide breaking API changes. Prefer declaring the fixtures using the actual
`postRouteReq` payload type (e.g. `RoutePostType` / `Parameters<typeof
postRouteReq>[1]`) and constructing the objects without `unknown`/`Record`
casts.
##########
src/types/schema/pageSearch.ts:
##########
@@ -19,16 +19,22 @@ import { z } from 'zod';
export const pageSearchSchema = z
.object({
- page: z
- .union([z.string(), z.number()])
- .optional()
- .default(1)
- .transform((val) => (val ? Number(val) : 1)),
- page_size: z
- .union([z.string(), z.number()])
- .optional()
- .default(10)
- .transform((val) => (val ? Number(val) : 10)),
+ page: z.preprocess(
+ (val) => {
+ if (val === undefined || val === null || val === '') return undefined;
+ const num = Number(val);
+ return Number.isNaN(num) || num <= 0 ? undefined : num;
+ },
+ z.number().int().min(1).optional().default(1)
+ ),
+ page_size: z.preprocess(
+ (val) => {
+ if (val === undefined || val === null || val === '') return undefined;
+ const num = Number(val);
+ return Number.isNaN(num) || num <= 0 ? undefined : num;
+ },
+ z.number().int().min(1).optional().default(10)
+ ),
Review Comment:
`page` / `page_size` preprocessing currently allows non-integer numeric
inputs (e.g. `page=1.5`) through the preprocess step, which then fails
`z.number().int()` and can cause `validateSearch` /
`pageSearchSchema.parse(...)` to throw for malformed URLs. Consider also
treating non-integer values as invalid in the preprocess (e.g. return
`undefined` when `!Number.isInteger(num)`) so the schema reliably falls back to
defaults instead of erroring.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
