Skip to content
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

fix: non-GET request endpoints #2233

Merged
merged 1 commit into from
Nov 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/qwik-city/middleware/request-handler/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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,
Expand Down
120 changes: 84 additions & 36 deletions packages/qwik-city/middleware/request-handler/user-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type {
RequestHandler,
PageModule,
RequestEvent,
ResponseContext,
ResponseContext as ResponseContextInterface,
RouteModule,
RouteParams,
} from '../../runtime/src/types';
Expand All @@ -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,
};

Expand Down Expand Up @@ -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++;

Expand All @@ -86,7 +81,7 @@ export async function loadUserResponse(

let reqHandler: RequestHandler | undefined = undefined;

switch (request.method) {
switch (method) {
case 'GET': {
reqHandler = endpointModule.onGet;
break;
Expand Down Expand Up @@ -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 = {
Expand All @@ -149,7 +130,7 @@ export async function loadUserResponse(
params: { ...params },
response,
platform,
cookie: userResponse.cookie,
cookie,
next,
abort,
};
Expand Down Expand Up @@ -189,7 +170,7 @@ export async function loadUserResponse(
userResponse.aborted = routeModuleIndex >= ABORT_INDEX;

if (
!isPageDataRequest &&
!isPageDataReq &&
isRedirectStatus(userResponse.status) &&
userResponse.headers.has('Location')
) {
Expand All @@ -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 {
Expand Down
111 changes: 109 additions & 2 deletions packages/qwik-city/middleware/request-handler/user-response.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();