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

Address routr api changes & Handle POST #837

Merged
merged 15 commits into from
Jan 18, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions docs/page-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ A binding of URL paths to a Page class that can render the results for those URL
* A string or regex that will be tested against the path of the URL to
determine if this route applies.

`method, optional: String | [String]`
`method, optional: undefined | String | [String]`

* The HTTP method(s) that are acceptable.
* The HTTP method(s) that are acceptable. Use an array of strings to specify multiple methods. Do not
set this value to `null` or `[]`. To match all HTTP methods, either leave out the parameter altogether or explicitly
set the value to `undefined`.

* Default: “get”
* Default: undefined

`page: Page`
* The page to render
Expand Down
20 changes: 13 additions & 7 deletions packages/react-server-cli/src/compileClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,14 +291,20 @@ module.exports = {
for (let routeName of Object.keys(routes.routes)) {
let route = routes.routes[routeName];

// On the line below specifying 'method', if the route doesn't have a method, we'll set it to `undefined` so that
// routr passes through and matches any method
// https://github.com/yahoo/routr/blob/v2.1.0/lib/router.js#L49-L57
let method = route.method;

// Safely check for an empty object, array, or string and specifically set it to 'undefined'
if (method === undefined || method === null || method === {} || method.length === 0) {
method = undefined; // 'undefined' is the value that routr needs to accept any method
}

routesOutput.push(`
${routeName}: {`);
routesOutput.push(`
path: ${JSON.stringify(route.path)},`);
// if the route doesn't have a method, we'll assume it's "get". routr doesn't
// have a default (method is required), so we set it here.
routesOutput.push(`
method: "${route.method || "get"}",`);
${routeName}: {
path: ${JSON.stringify(route.path)},
method: ${JSON.stringify(method)},`);

let formats = normalizeRoutesPage(route.page);
routesOutput.push(`
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export default class DumbPage {
getElements() {
const routeName = this.getRequest().getRouteName();
return <div id="routeName">{routeName}</div>;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import DumbPage from "./DumbPage";

// This wrapper is taken from the compileClient function that writes the routes_(client|server).js file
const pageWrapper = {
default: () => {
return {
done: (cb) => { cb(DumbPage); },
};
},
};

const Routes = {
"GetPage": {
"path": ["/getPage"],
"page": pageWrapper,
"method": "get",
},
"HeadPage": {
"path": ["/headPage"],
"page": pageWrapper,
"method": "head",
},
"PostPage": {
"path": ["/postPage"],
"page": pageWrapper,
"method": "post",
},
"PatchPage": {
"path": ["/patchPage"],
"page": pageWrapper,
"method": "patch",
},
"PutPage": {
"path": ["/putPage"],
"page": pageWrapper,
"method": "put",
},
"GetPageCaps": {
"path": ["/getPageCaps"],
"page": pageWrapper,
"method": "GET", // this should be all caps because the req.getMethod() will return 'get'.
},
"GetAndPostPage": {
"path": ["/getAndPostPage"],
"page": pageWrapper,
"method": ["get", "post"],
},
"AllMethodsPage": {
"path": ["/allMethodsPage"],
"page": pageWrapper,
"method": undefined,
},
"NullMethodsPage": {
"path": ["/nullMethodsPage"],
"page": pageWrapper,
"method": null,
},
"EmptyArrayMethodsPage": {
"path": ["/emptyArrayMethodsPage"],
"page": pageWrapper,
"method": [],
},

};

export default Routes;
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
import Navigator from "../../../context/Navigator";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THANK YOU FOR THESE TESTS!

import History from "../../../components/History";
import ExpressServerRequest from "../../../ExpressServerRequest";
import NavigatorRoutes from "./NavigatorRoutes";

class RequestContextStub {
constructor(options) {
this.navigator = new Navigator(this, options);
}
navigate (request, type) {
this.navigator.navigate(request, type);
}
framebackControllerWillHandle() { return false; }
getMobileDetect() { return null; }
}


describe("Navigator", () => {
let requestContext;
const options = {
routes: NavigatorRoutes,
};

beforeEach(() => {
requestContext = new RequestContextStub(options);
});
afterEach(() => {
requestContext = null;
});

it("routes to a page using a method GET (caps)", (done) => {
const req = {
method: "get",
protocol: "http",
secure: false,
hostname: "localhost",
url: "/getPageCaps",
};
const expressRequest = new ExpressServerRequest(req);

const navigatedToPage = () => {
expect(expressRequest.getRouteName()).toBe('GetPageCaps');
done();
};
requestContext.navigator.on('navigateDone', navigatedToPage);
requestContext.navigator.on('page', navigatedToPage);

requestContext.navigate(expressRequest, History.events.PAGELOAD);
});

it("routes to a page that can handle GET and POST using a method GET", (done) => {
const req = {
method: "get",
protocol: "http",
secure: false,
hostname: "localhost",
url: "/getAndPostPage",
};
const expressRequest = new ExpressServerRequest(req);

const navigatedToPage = () => {
expect(expressRequest.getRouteName()).toBe('GetAndPostPage');
done();
};
requestContext.navigator.on('navigateDone', navigatedToPage);
requestContext.navigator.on('page', navigatedToPage);

requestContext.navigate(expressRequest, History.events.PAGELOAD);
});

it("routes to a page that can handle GET and POST using a method POST", (done) => {
const req = {
method: "post",
protocol: "http",
secure: false,
hostname: "localhost",
url: "/getAndPostPage",
};
const expressRequest = new ExpressServerRequest(req);

const navigatedToPage = () => {
expect(expressRequest.getRouteName()).toBe('GetAndPostPage');
done();
};
requestContext.navigator.on('navigateDone', navigatedToPage);
requestContext.navigator.on('page', navigatedToPage);

requestContext.navigate(expressRequest, History.events.PAGELOAD);
});


const methods = [
'get',
'head',
'put',
'patch',
'post',
];

methods.forEach((testingMethod) => {
methods.forEach((otherMethod) => {
const req = {
method: otherMethod,
protocol: "http",
secure: false,
hostname: "localhost",
url: `/${testingMethod}Page`,
};
const expressRequest = new ExpressServerRequest(req);

if (testingMethod === otherMethod) {
it(`does route to a page expecting ${otherMethod} using a method ${testingMethod}`, (done) => {
const navigatedToPage = (err) => {
const httpStatus = err.status || 200;
expect(httpStatus).toBe(200, `A route with method ${testingMethod} was not found when one should have been found.`);
done();
};
requestContext.navigator.on('navigateDone', navigatedToPage);
requestContext.navigator.on('page', navigatedToPage);
requestContext.navigate(expressRequest, History.events.PAGELOAD);
});

it("routes to a page that accepts all methods", (done) => {
const allMethodReq = {
method: otherMethod,
protocol: "http",
secure: false,
hostname: "localhost",
url: "/allMethodsPage",
};
const allMethodExpressRequest = new ExpressServerRequest(allMethodReq);

const navigatedToPage = (err) => {
const httpStatus = err.status || 200;
expect(httpStatus).toBe(200, `A route with method ${testingMethod} was not found for the AllMethodsPage.`);
expect(allMethodExpressRequest.getRouteName()).toBe('AllMethodsPage');
done();
};
requestContext.navigator.on('navigateDone', navigatedToPage);
requestContext.navigator.on('page', navigatedToPage);

requestContext.navigate(allMethodExpressRequest, History.events.PAGELOAD);
});

it("doesn't route to a page because the methods were set improperly with a 'null'", (done) => {
const noMethodReq = {
method: otherMethod,
protocol: "http",
secure: false,
hostname: "localhost",
url: "/nullMethodsPage",
};
const noMethodExpressRequest = new ExpressServerRequest(noMethodReq);

const navigatedToPage = (err) => {
const httpStatus = err.status || 200;
expect(httpStatus).toBe(404, `A route with method ${testingMethod} was found for the NullMethodsPage.`);
done();
};
requestContext.navigator.on('navigateDone', navigatedToPage);
requestContext.navigator.on('page', navigatedToPage);

requestContext.navigate(noMethodExpressRequest, History.events.PAGELOAD);
});

it("doesn't route to a page because the methods were set improperly with a '[]'", (done) => {
const noMethodReq = {
method: otherMethod,
protocol: "http",
secure: false,
hostname: "localhost",
url: "/emptyArrayMethodsPage",
};
const noMethodExpressRequest = new ExpressServerRequest(noMethodReq);

const navigatedToPage = (err) => {
const httpStatus = err.status || 200;
expect(httpStatus).toBe(404, `A route with method ${testingMethod} was found for the EmptyArrayMethodsPage.`);
done();
};
requestContext.navigator.on('navigateDone', navigatedToPage);
requestContext.navigator.on('page', navigatedToPage);

requestContext.navigate(noMethodExpressRequest, History.events.PAGELOAD);
});
} else {
it(`doesn't route to a page expecting ${otherMethod} using a method ${testingMethod}`, (done) => {
const navigatedToPage = (err) => {
const httpStatus = err.status || 200;
expect(httpStatus).toBe(404, `A route with method ${testingMethod} was found when one should not have been found.`);
done();
};
requestContext.navigator.on('navigateDone', navigatedToPage);
requestContext.navigator.on('page', navigatedToPage);
requestContext.navigate(expressRequest, History.events.PAGELOAD);
});
}
});

});

});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some failure cases here too? E.g. doesn't respond to a POST request when configured with PUT only &c.

2 changes: 1 addition & 1 deletion packages/react-server/core/context/Navigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class Navigator extends EventEmitter {

this._haveInitialized = true;

var route = this.router.getRoute(request.getUrl(), {navigate: {path:request.getUrl(), type:type}});
var route = this.router.getRoute(request.getUrl(), { method: request.getMethod() });

if (route) {
logger.debug(`Mapped ${request.getUrl()} to route ${route.name}`);
Expand Down