Skip to content

Commit

Permalink
fix: don't cache the intermediate locals for application (eggjs#1146)
Browse files Browse the repository at this point in the history
  • Loading branch information
JacksonTian authored and fengmk2 committed Jul 4, 2017
1 parent 7c70beb commit b80bb14
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 8 deletions.
12 changes: 4 additions & 8 deletions lib/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const { assign } = require('utility');
const KEYS = Symbol('Application#keys');
const HELPER = Symbol('Application#Helper');
const LOCALS = Symbol('Application#locals');
const LOCALS_LIST = Symbol('Application#localsList');
const BIND_EVENTS = Symbol('Application#bindEvents');
const WARN_CONFUSED_CONFIG = Symbol('Application#warnConfusedConfig');
const EGG_LOADER = Symbol.for('egg#loader');
Expand Down Expand Up @@ -76,18 +75,15 @@ class Application extends EggApplication {
if (!this[LOCALS]) {
this[LOCALS] = {};
}
if (this[LOCALS_LIST] && this[LOCALS_LIST].length) {
assign(this[LOCALS], this[LOCALS_LIST]);
this[LOCALS_LIST] = null;
}
return this[LOCALS];
}

set locals(val) {
if (!this[LOCALS_LIST]) {
this[LOCALS_LIST] = [];
if (!this[LOCALS]) {
this[LOCALS] = {};
}
this[LOCALS_LIST].push(val);

assign(this[LOCALS], val);
}

/**
Expand Down
26 changes: 26 additions & 0 deletions test/app/extend/application.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,32 @@ describe('test/app/extend/application.test.js', () => {
.get('/app_same_ref')
.expect('true');
});

it('should app.locals not OOM', () => {
return app.httpRequest()
.get('/app_locals_oom')
.expect('ok');
});
});

describe('app.locals.foo = bar', () => {
let app;
before(() => {
app = utils.app('apps/app-locals-getter');
return app.ready();
});
after(() => app.close());

it('should work', () => {
return app.httpRequest()
.get('/test')
.expect({
locals: {
foo: 'bar',
abc: '123',
},
});
});
});

describe('app.createAnonymousContext()', () => {
Expand Down
11 changes: 11 additions & 0 deletions test/fixtures/apps/app-locals-getter/app/router.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';

module.exports = app => {
app.get('/test', function* () {
this.app.locals.foo = 'bar';
this.locals.abc = '123';
this.body = {
locals: this.locals,
};
});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports.keys = 'foo';
3 changes: 3 additions & 0 deletions test/fixtures/apps/app-locals-getter/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "app-locals-getter"
}
10 changes: 10 additions & 0 deletions test/fixtures/apps/locals/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ module.exports = app => {
this.body = app1 === app2;
});

app.get('/app_locals_oom', function*() {
for (let i = 0; i < 1000; i++) {
app.locals = {
// 10MB
buff: Buffer.alloc(10 * 1024 * 1024).toString()
};
}
this.body = 'ok';
});

app.get('/ctx_same_ref', function*() {
let ctx1, ctx2;
this.locals = {
Expand Down

0 comments on commit b80bb14

Please sign in to comment.