fix: non-GET request endpoints (#2233)

QwikDev · Nov 22, 2022 · c8aec8d · c8aec8d
1 parent dc91466
commit c8aec8d
Show file tree

Hide file tree

Showing 6 changed files with 288 additions and 54 deletions.
diff --git a/packages/qwik-city/middleware/request-handler/test-utils.ts b/packages/qwik-city/middleware/request-handler/test-utils.ts
@@ -6,6 +6,7 @@ import type { QwikCityRequestContext, ResponseHandler } from './types';
 export function mockRequestContext(opts?: {
   method?: string;
   url?: string | URL;
+  headers?: Record<string, string>;
 }): TestQwikCityRequestContext {
   const url = new URL(opts?.url || '/', 'https://qwik.builder.io');
 
@@ -17,6 +18,11 @@ export function mockRequestContext(opts?: {
     json: () => Promise.resolve({}),
     text: () => Promise.resolve(''),
   };
+  if (opts?.headers) {
+    for (const key in opts.headers) {
+      request.headers.set(key, opts.headers[key]);
+    }
+  }
 
   const responseData: { status: number; headers: Headers; body: Promise<string> } = {
     status: 200,

diff --git a/packages/qwik-city/middleware/request-handler/user-response.ts b/packages/qwik-city/middleware/request-handler/user-response.ts
@@ -3,7 +3,7 @@ import type {
   RequestHandler,
   PageModule,
   RequestEvent,
-  ResponseContext,
+  ResponseContext as ResponseContextInterface,
   RouteModule,
   RouteParams,
 } from '../../runtime/src/types';
@@ -26,19 +26,22 @@ export async function loadUserResponse(
 
   const { request, url, platform } = requestCtx;
   const { pathname } = url;
+  const { method, headers } = request;
   const isPageModule = isLastModulePageRoute(routeModules);
-  const isPageDataRequest = isPageModule && request.headers.get('Accept') === 'application/json';
-  const type = isPageDataRequest ? 'pagedata' : isPageModule ? 'pagehtml' : 'endpoint';
+  const isEndpointReq = isEndPointRequest(method, headers.get('Accept'));
+  const isPageDataReq = isPageModule && isEndpointReq;
+
+  const cookie = new Cookie(headers.get('cookie'));
 
   const userResponse: UserResponseContext = {
-    type,
+    type: isPageDataReq ? 'pagedata' : isPageModule ? 'pagehtml' : 'endpoint',
     url,
     params,
     status: HttpStatus.Ok,
     headers: createHeaders(),
     resolvedBody: undefined,
     pendingBody: undefined,
-    cookie: new Cookie(request.headers.get('cookie')),
+    cookie,
     aborted: false,
   };
 
@@ -70,14 +73,6 @@ export async function loadUserResponse(
     routeModuleIndex = ABORT_INDEX;
   };
 
-  const redirect = (url: string, status?: number) => {
-    return new RedirectResponse(url, status, userResponse.headers, userResponse.cookie);
-  };
-
-  const error = (status: number, message?: string) => {
-    return new ErrorResponse(status, message);
-  };
-
   const next = async () => {
     routeModuleIndex++;
 
@@ -86,7 +81,7 @@ export async function loadUserResponse(
 
       let reqHandler: RequestHandler | undefined = undefined;
 
-      switch (request.method) {
+      switch (method) {
         case 'GET': {
           reqHandler = endpointModule.onGet;
           break;
@@ -122,25 +117,11 @@ export async function loadUserResponse(
       if (typeof reqHandler === 'function') {
         hasRequestMethodHandler = true;
 
-        const response: ResponseContext = {
-          get status() {
-            return userResponse.status;
-          },
-          set status(code) {
-            userResponse.status = code;
-          },
-          get headers() {
-            return userResponse.headers;
-          },
-          get locale() {
-            return requestCtx.locale;
-          },
-          set locale(locale) {
-            requestCtx.locale = locale;
-          },
-          redirect,
-          error,
-        };
+        if (isEndpointReq && method !== 'GET') {
+          userResponse.type = 'endpoint';
+        }
+
+        const response = new ResponseContext(userResponse, requestCtx);
 
         // create user request event, which is a narrowed down request context
         const requestEv: RequestEvent = {
@@ -149,7 +130,7 @@ export async function loadUserResponse(
           params: { ...params },
           response,
           platform,
-          cookie: userResponse.cookie,
+          cookie,
           next,
           abort,
         };
@@ -189,7 +170,7 @@ export async function loadUserResponse(
   userResponse.aborted = routeModuleIndex >= ABORT_INDEX;
 
   if (
-    !isPageDataRequest &&
+    !isPageDataReq &&
     isRedirectStatus(userResponse.status) &&
     userResponse.headers.has('Location')
   ) {
@@ -204,13 +185,80 @@ export async function loadUserResponse(
   }
 
   // this is only an endpoint, and not a page module
-  if (type === 'endpoint' && !hasRequestMethodHandler) {
+  if (userResponse.type === 'endpoint' && !hasRequestMethodHandler) {
     // didn't find any handlers
     throw new ErrorResponse(HttpStatus.MethodNotAllowed, `Method Not Allowed`);
   }
   return userResponse;
 }
 
+const UserRsp = Symbol('UserResponse');
+const RequestCtx = Symbol('RequestContext');
+
+class ResponseContext implements ResponseContextInterface {
+  [UserRsp]: UserResponseContext;
+  [RequestCtx]: QwikCityRequestContext;
+
+  constructor(userResponse: UserResponseContext, requestCtx: QwikCityRequestContext) {
+    this[UserRsp] = userResponse;
+    this[RequestCtx] = requestCtx;
+  }
+  get status() {
+    return this[UserRsp].status;
+  }
+  set status(code) {
+    this[UserRsp].status = code;
+  }
+  get headers() {
+    return this[UserRsp].headers;
+  }
+  get locale() {
+    return this[RequestCtx].locale;
+  }
+  set locale(locale) {
+    this[RequestCtx].locale = locale;
+  }
+  redirect(url: string, status?: number) {
+    return new RedirectResponse(url, status, this[UserRsp].headers, this[UserRsp].cookie);
+  }
+  error(status: number, message?: string) {
+    return new ErrorResponse(status, message);
+  }
+}
+
+export function isEndPointRequest(method: string, acceptHeader: string | null) {
+  if (method === 'GET' || method === 'POST') {
+    // further check if GET or POST is an endpoint request
+    // check if there's an Accept request header
+    if (acceptHeader) {
+      const htmlIndex = acceptHeader.indexOf('text/html');
+      if (htmlIndex === 0) {
+        // starts with text/html
+        // not an endpoint GET/POST request
+        return false;
+      }
+
+      const jsonIndex = acceptHeader.indexOf('application/json');
+      if (jsonIndex > -1) {
+        // has application/json Accept header
+        if (htmlIndex > -1) {
+          // if application/json before text/html
+          // then it's an endpoint GET/POST request
+          return jsonIndex < htmlIndex;
+        }
+        return true;
+      }
+    }
+
+    // not an endpoint GET/POST request
+    return false;
+  } else {
+    // always endpoint for non-GET/POST request
+    // PUT, PATCH, DELETE, OPTIONS, HEAD, etc
+    return true;
+  }
+}
+
 function createPendingBody(cb: () => any) {
   return new Promise<any>((resolve, reject) => {
     try {

diff --git a/packages/qwik-city/middleware/request-handler/user-response.unit.ts b/packages/qwik-city/middleware/request-handler/user-response.unit.ts
@@ -2,11 +2,31 @@ import { test } from 'uvu';
 import { equal, instance } from 'uvu/assert';
 import { mockRequestContext, wait } from './test-utils';
 import type { PageModule, RouteModule } from '../../runtime/src/types';
-import { loadUserResponse, updateRequestCtx } from './user-response';
+import { isEndPointRequest, loadUserResponse, updateRequestCtx } from './user-response';
 import { RedirectResponse } from './redirect-handler';
 import { ErrorResponse } from './error-handler';
 
-test('endpoint type cuz no default module export', async () => {
+test('endpoint type cuz non-get method, and method handler, and with default module export', async () => {
+  const requestCtx = mockRequestContext({
+    method: 'POST',
+    headers: {
+      Accept: 'application/json',
+    },
+  });
+  const trailingSlash = false;
+
+  const endpoints: PageModule[] = [
+    {
+      onPost: () => {},
+      default: () => {},
+    },
+  ];
+
+  const u = await loadUserResponse(requestCtx, {}, endpoints, trailingSlash);
+  equal(u.type, 'endpoint');
+});
+
+test('endpoint type cuz and default module export', async () => {
   const requestCtx = mockRequestContext();
   const trailingSlash = false;
 
@@ -270,4 +290,91 @@ test('updateRequestCtx, root, no trailing slash', () => {
   equal(requestCtx.request.headers.get('Accept'), 'application/json');
 });
 
+[
+  {
+    method: 'PUT',
+    acceptHeader: 'text/html',
+    expect: true,
+  },
+  {
+    method: 'PUT',
+    acceptHeader: null,
+    expect: true,
+  },
+  {
+    method: 'HEAD',
+    acceptHeader: null,
+    expect: true,
+  },
+  {
+    method: 'DELETE',
+    acceptHeader: null,
+    expect: true,
+  },
+  {
+    method: 'PATCH',
+    acceptHeader: null,
+    expect: true,
+  },
+  {
+    method: 'POST',
+    acceptHeader: null,
+    expect: false,
+  },
+  {
+    method: 'POST',
+    acceptHeader: 'application/json',
+    expect: true,
+  },
+  {
+    method: 'POST',
+    acceptHeader: 'application/json,text/html',
+    expect: true,
+  },
+  {
+    method: 'POST',
+    acceptHeader: 'text/html, application/json',
+    expect: false,
+  },
+  {
+    method: 'POST',
+    acceptHeader: 'text/html',
+    expect: false,
+  },
+  {
+    method: 'GET',
+    acceptHeader: 'application/json',
+    expect: true,
+  },
+  {
+    method: 'GET',
+    acceptHeader: 'application/json,text/html',
+    expect: true,
+  },
+  {
+    method: 'GET',
+    acceptHeader: 'text/html, application/json',
+    expect: false,
+  },
+  {
+    method: 'GET',
+    acceptHeader: 'text/html',
+    expect: false,
+  },
+  {
+    method: 'GET',
+    acceptHeader: '*/*',
+    expect: false,
+  },
+  {
+    method: 'GET',
+    acceptHeader: null,
+    expect: false,
+  },
+].forEach((t) => {
+  test(`isEndPointRequest ${t.method}, Accept: ${t.acceptHeader}`, () => {
+    equal(isEndPointRequest(t.method, t.acceptHeader), t.expect);
+  });
+});
+
 test.run();