Skip to content

Reverts calling next() after handling response #2634

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

Merged
merged 4 commits into from
Sep 9, 2016
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
7 changes: 6 additions & 1 deletion spec/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ let openConnections = {};
var app = express();
var api = new ParseServer(defaultConfiguration);
app.use('/1', api);
app.use('/1', (req, res) => {
fail('should not call next');
});
var server = app.listen(port);
server.on('connection', connection => {
let key = `${connection.remoteAddress}:${connection.remotePort}`;
Expand All @@ -126,7 +129,9 @@ const reconfigureServer = changedConfiguration => {
api = new ParseServer(newConfiguration);
api.use(require('./testing-routes').router);
app.use('/1', api);

app.use('/1', (req, res) => {
fail('should not call next');
});
server = app.listen(port);
server.on('connection', connection => {
let key = `${connection.remoteAddress}:${connection.remotePort}`;
Expand Down
8 changes: 1 addition & 7 deletions src/Controllers/AnalyticsController.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import AdaptableController from './AdaptableController';
import { AnalyticsAdapter } from '../Adapters/Analytics/AnalyticsAdapter';

const AppOpenedEventName = 'AppOpened';

export class AnalyticsController extends AdaptableController {
appOpened(req) {
return Promise.resolve().then(() => {
Expand All @@ -15,12 +13,8 @@ export class AnalyticsController extends AdaptableController {
}

trackEvent(req) {
const eventName = req.params.eventName;
if (eventName === AppOpenedEventName) {
return this.appOpened(req);
}
return Promise.resolve().then(() => {
return this.adapter.trackEvent(eventName, req.body, req);
return this.adapter.trackEvent(req.params.eventName, req.body, req);
}).then((response) => {
return { response: response || {} };
}).catch((err) => {
Expand Down
4 changes: 2 additions & 2 deletions src/ParseServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,9 @@ class ParseServer {
api.use('/', bodyParser.urlencoded({extended: false}), new PublicAPIRouter().expressRouter());

api.use(bodyParser.json({ 'type': '*/*' , limit: maxUploadSize }));
api.use(middlewares.allowCrossDomain);
api.use(middlewares.allowMethodOverride);
api.use(middlewares.handleParseHeaders);

let appRouter = ParseServer.promiseRouter({ appId });
api.use(appRouter.expressRouter());
Expand Down Expand Up @@ -322,8 +324,6 @@ class ParseServer {
}, []);

let appRouter = new PromiseRouter(routes, appId);
appRouter.use(middlewares.allowCrossDomain);
appRouter.use(middlewares.handleParseHeaders);

batch.mountOnto(appRouter);
return appRouter;
Expand Down
13 changes: 3 additions & 10 deletions src/PromiseRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export default class PromiseRouter {
// location: optional. a location header
constructor(routes = [], appId) {
this.routes = routes;
this.middlewares = [];
this.appId = appId;
this.mountRoutes();
}
Expand All @@ -55,10 +54,6 @@ export default class PromiseRouter {
}
};

use(middleware) {
this.middlewares.push(middleware);
}

route(method, path, ...handlers) {
switch(method) {
case 'POST':
Expand Down Expand Up @@ -117,8 +112,7 @@ export default class PromiseRouter {
this.routes.forEach((route) => {
let method = route.method.toLowerCase();
let handler = makeExpressHandler(this.appId, route.handler);
let args = [].concat(route.path, this.middlewares, handler);
expressApp[method].apply(expressApp, args);
expressApp[method].call(expressApp, route.path, handler);
});
return expressApp;
};
Expand Down Expand Up @@ -175,7 +169,7 @@ function makeExpressHandler(appId, promiseHandler) {

if (result.text) {
res.send(result.text);
return next();
return;
}

if (result.location) {
Expand All @@ -184,7 +178,7 @@ function makeExpressHandler(appId, promiseHandler) {
// as it double encodes %encoded chars in URL
if (!result.response) {
res.send('Found. Redirecting to '+result.location);
return next();
return;
}
}
if (result.headers) {
Expand All @@ -193,7 +187,6 @@ function makeExpressHandler(appId, promiseHandler) {
})
}
res.json(result.response);
next();
}, (e) => {
log.error(`Error generating response. ${inspect(e)}`, {error: e});
next(e);
Expand Down
6 changes: 6 additions & 0 deletions src/Routers/AnalyticsRouter.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
// AnalyticsRouter.js
import PromiseRouter from '../PromiseRouter';

function appOpened(req) {
const analyticsController = req.config.analyticsController;
return analyticsController.appOpened(req);
}

function trackEvent(req) {
const analyticsController = req.config.analyticsController;
return analyticsController.trackEvent(req);
Expand All @@ -9,6 +14,7 @@ function trackEvent(req) {

export class AnalyticsRouter extends PromiseRouter {
mountRoutes() {
this.route('POST','/events/AppOpened', appOpened);
this.route('POST','/events/:eventName', trackEvent);
}
}
4 changes: 1 addition & 3 deletions src/Routers/SessionsRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ export class SessionsRouter extends ClassesRouter {
}

handleGet(req) {
if (req.params.objectId === 'me') {
return this.handleMe(req);
}
req.params.className = '_Session';
return super.handleGet(req);
}
Expand Down Expand Up @@ -86,6 +83,7 @@ export class SessionsRouter extends ClassesRouter {
}

mountRoutes() {
this.route('GET','/sessions/me', req => { return this.handleMe(req); });
this.route('GET', '/sessions', req => { return this.handleFind(req); });
this.route('GET', '/sessions/:objectId', req => { return this.handleGet(req); });
this.route('POST', '/sessions', req => { return this.handleCreate(req); });
Expand Down
4 changes: 1 addition & 3 deletions src/Routers/UsersRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ export class UsersRouter extends ClassesRouter {
}

handleGet(req) {
if (req.params.objectId === 'me') {
return this.handleMe(req);
}
req.params.className = '_User';
return super.handleGet(req);
}
Expand Down Expand Up @@ -214,6 +211,7 @@ export class UsersRouter extends ClassesRouter {
mountRoutes() {
this.route('GET', '/users', req => { return this.handleFind(req); });
this.route('POST', '/users', req => { return this.handleCreate(req); });
this.route('GET', '/users/me', req => { return this.handleMe(req); });
this.route('GET', '/users/:objectId', req => { return this.handleGet(req); });
this.route('PUT', '/users/:objectId', req => { return this.handleUpdate(req); });
this.route('DELETE', '/users/:objectId', req => { return this.handleDelete(req); });
Expand Down