eggjs / egg

🥚 Born to build better enterprise frameworks and apps with Node.js & Koa
https://eggjs.org
MIT License
18.88k stars 1.81k forks source link

希望在middleware中能拿到当前请求会映射到什么controller的什么action #347

Closed AnzerWall closed 7 years ago

AnzerWall commented 7 years ago

当前只能改变router才能实现,然而得继承框架。。。

这样实现类似于控制器级别中间件,类似于thinkjs的__before. action级别自动validate之类,类似于thinkjs的logic层。

popomore commented 7 years ago

https://eggjs.org/zh-cn/basics/router.html

路由支持中间价 AnzerWall notifications@github.com于2017年2月10日 周五00:05写道:

当前只能改变router才能实现,然而得继承框架。。。

这样实现类似于控制器级别中间件,类似于thinkjs的__before. action级别自动validate之类,类似于thinkjs的logic层。

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/eggjs/egg/issues/347, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWA1ddPRrIbaQcvvdip_SSwohcSM8Scks5razlQgaJpZM4L8UMx .

fengmk2 commented 7 years ago

middleware 入侵 controller 逻辑味道有点怪怪的。

AnzerWall commented 7 years ago

我知道路由支持中间件。 但是Validator逻辑又是类似,都是加载schema,失败就返回特定的响应。 如果可以拿到对应的controller以及action,那么我可以写同名文件的Validator,自动对特定的方法做校验。。而不用在router指定。。 一般同一个controller的逻辑也是相关的,可能就需要控制器级别的中间件。。

其实就是把router拆成两个,一个解析url,ctx写入映射结果,比如actionName,controllerName,一个根据这个调用不同的controller的action。。。

比如权限控制,可以action级别

其实就是不想写两遍类似router的东西

popomore commented 7 years ago

可以转换下思路,我们习惯把 API 定义在 ctx 上,可以参考这个插件 https://github.com/eggjs/egg-validate AnzerWall notifications@github.com于2017年2月10日 周五00:26写道:

我知道路由支持中间件。 但是Validator逻辑又是类似,都是加载schema,失败就返回特定的响应。

如果可以拿到对应的controller以及action,那么我可以写同名文件的Validator,自动对特定的方法做校验。。而不用在router指定。。 一般同一个controller的逻辑也是相关的,可能就需要控制器级别的中间件。。

其实就是把router拆成两个,一个解析url,ctx写入映射结果,比如actionName,controllerName,一个根据这个调用不同的controller的action。。。

比如权限控制,可以action级别

其实就是不想写两遍类似router的东西

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/eggjs/egg/issues/347#issuecomment-278693216, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWA1QWZjmxImHLIJIjIdIACixtK30Pmks5raz4PgaJpZM4L8UMx .

AnzerWall commented 7 years ago

这样会有大量重复的this.ctx.validate(rule)的重复逻辑, 像是例子里schema写在controller的时候,比如我使用joi做校验,有时候schema比较长代码就比较肿。 这时候就有拆分validator层的扩展。。

再假设以后支持装饰器,或者使用ts的用户。。 类似@permission("User.create") 或者@GET("/users") 这样的逻辑该怎么扩展呢。

dead-horse commented 7 years ago

@AnzerWall 你可以理解为你只需要实现一个中间件,然后在需要的 router 中引入,例如:

// app/middleware/validate.js
module.exports = options => {
  const validator = createValidator(options);
  return function* (next) {
    validator(this.request.body);
    yield next;
  };
};

在 router 中引入:

// app/router.js

app.post('/api/users', this.middlewares.validate('user.create'), 'user.create');
app.post('/api/blogs', this.middlewares.validate('blog.create'), 'blog.create');

至于装饰器,其实也可以把它当做一个中间件来看待, @AnzerWall 可以代入 express / koa 的思维解决问题,强行套 thinkjs 的思维可能会绕弯路。

dead-horse commented 7 years ago

另外,所有决定映射到哪个 controller 的 action 的都是 url path,所以在中间件中完全可以基于 url path 就实现你做什么 validate 的需求了。

AnzerWall commented 7 years ago

@dead-horse

实现中间件的思路,上面说了,我每一个router我都得手动指定。(总归原因的程序员的懒)

第二个回复是指,middleware的match吗,可是我又怎么怎么知道映射关系呢,能不能不多写一份映射吗,毕竟egg 奉行『约定优于配置』,约定写同文件名的validator行不行

至于装饰器,我想知道,我写了一个装饰器,装饰在action上,能拿到的信息是类名方法名,我该怎么知道映射到的url,比如@Permission('user.create')? 要写成@Permission('user.create','/users','POST')吗?

AnzerWall commented 7 years ago

现在我的的那个需求的暂时解决思路是,app.post('/api/users',..._('user.create')),让_返回中间件数组了。。。

AnzerWall commented 7 years ago

@dead-horse 哎呀,也不是套thinkjs的思想,毕竟这个方式有点方便。 你不要这样就把我批判一遍嘛,虽然我使用过一段时间,但我也不是思想僵化的人。。

atian25 commented 7 years ago

提供一种思路,太晚了,明天再看看。

自定义加载这个通过 loader 的 api 很容易可以做到加载 app/validate/*.js 映射为 app.validateRule.xxx,然后实现自定义的 app.Controller 基类,在这里面想办法自动 hook 同名 rule ?如 proxy 之类的?

发自我的 iPhone

在 2017年2月10日,01:19,AnzerWall notifications@github.com 写道:

现在的暂时解决思路是,app.post('/api/users',_('user.create')); 让_返回中间件数组了。。。

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

AnzerWall commented 7 years ago

@atian25 这是最开始的思路,但是Loader 的扩展只能在框架进行,得屏蔽掉router的加载,自己撸一套。。

AnzerWall commented 7 years ago

@atian25 抱歉,我理解错你的思路,你是说proxy,permission也可以类似的思路实现,是个思路、。。。 @GET 装饰器就不好做了,不过,这也是集中路由和分散路由的区别

atian25 commented 7 years ago

loader 不需要在框架,在应用或插件的 app.js 中即可,可以看下 egg-view-nunjucks 中加载 filter 的实现

发自我的 iPhone

在 2017年2月10日,01:40,AnzerWall notifications@github.com 写道:

@atian25 这是最开始的思路,但是Loader 的扩展只能在框架进行,得屏蔽掉router的加载,自己撸一套。。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

AnzerWall commented 7 years ago

@atian25 恩,知道,我是指如果要实现@GET的话,想分散路由的话。。

哦,倒是可以注解中先集中路由信息,等加载controller,我往router再一个个app.get app.post之类,毕竟app没在global

AnzerWall commented 7 years ago

@atian25 感谢思路,提供了一定的比较好的解决方式吗,new Proxy()的话。。。。

atian25 commented 7 years ago

@AnzerWall 重新看了下, 几点思考:

controller 和 validate rule 的分离

controller 的职责一般是:

class NewsController extend app.Controller {
  * list() {
     const ctx = this.ctx;
     // 获取用户通过 HTTP 传递过来的请求参数。
     const page = parseInt(ctx.query.page) || 1;
     // 校验、组装参数
     ctx.valadate({
       page: 'number', 
     }, ctx.query);
     // 调用 service 进行业务处理
     const idList = yieldctx.service.hackerNews.getTopStories(page);
     // 通过 HTTP 将结果响应给用户。
     yield ctx.render('news/list.tpl', { list: idList });
  }
}

我们可以先讨论下这点, 「是否有必要把校验的规则放到独立的文件」?

如果结论是必要,那我们再来讨论如何做,这块我觉得问题不大, middleware,wrap router method,decorate,proxy 等等,都是属于语法糖方面的封装。

popomore commented 7 years ago

感觉这个不应该 egg 考虑的,应用层的使用差异很大。而且 validate 这种不是所有 controller 都需要的,所以还是需要一个地方来指定是否做校验。

@AnzerWall 可以先写下你的代码,我们再看如何组织。

atian25 commented 7 years ago

嗯, @AnzerWall 可以参考 #322 那样, 写下你的使用场景和伪代码, 我们再来讨论会清晰点.

@popomore 我猜测他的意思是有跟 controller action 同名的 rule 时, 自动校验, 没有的时候就跳过.

popomore commented 7 years ago

目录结构

`- app
  |- controller
  |  `- user.js
  `- validator
     `- user.js 

文件定义

// app/controller/user.js
module.exports = app => {
  return class UserController extends app.Controller {
    create() {}
  }
};

// app/validator/user.js

exports.create = { 规则 };

是不是按这种匹配规则,如果在 validator 定义了,就优先校验?

atian25 commented 7 years ago

嗯, 昨晚我理解这个 issue 的想法是这样的.

但早上起来想了下, 如上面提到, 感觉分离后反而不直观.

规则一般没太多代码的, 一些通过的规则, 通过 addRule 在 app.js 那边定义了就完了.

dead-horse commented 7 years ago

这样来做是不太够用的,这个就和之前 @huacnlee 提的权限校验一样,到后期很可能就需要更灵活的校验了,例如:

// controller
exports.update = function*(ctx) {
  const prepareData = yield ctx.service.user.get(ctx.params.id);
  // 在执行一些业务逻辑后再做 validate
  ctx.validate(rule, { prepareData, ctx.request.body});
}
atian25 commented 7 years ago

ping @AnzerWall

AnzerWall commented 7 years ago

@atian25 抱歉,这几天元宵出去浪了。。

controller 和 validate rule 的分离

rule文件分离是基于一种需求,分离静态的参数校验和动态的参数校验。。 有些参数是可以通过静态rule去检验,而有些参数是需要查询数据库才知道合法性。。。

那么我会在rules文件中写静态的rules,在controller中写动态的参数校验。

分离rules,然后在controller中可以可以很放心地假设·数据类型上合法·。

//validator/file-type.js
module.exports = function (Joi) {
    return {
        create: {
            body: Joi.object().keys({
                fileTypeName: Joi.string().regex(/^[a-zA-Z0-9\-_]{1,50}$/).required(),
                alias: Joi.string().default(Joi.ref('fileTypeName')),
                params: Joi.array().items({
                    style: Joi.string().valid(stypeList).required(),
                    name: Joi.string().required(),
                    isNumber: Joi.boolean().default(false),
                    alias: Joi.string().default(Joi.ref('name')),
                    required: Joi.boolean().default(false),
                    limitStringMinLength: Joi.boolean().default(false),
                    limitStringMaxLength: Joi.boolean().default(false),
                    limitNumberMin: Joi.boolean().default(false),
                    limitNumberMax: Joi.boolean().default(false),
                    limitSelectedMin: Joi.boolean().default(false),
                    limitSelectedMax: Joi.boolean().default(false),
                    limitDateTimeMin: Joi.boolean().default(false),
                    limitDateTimeMax: Joi.boolean().default(false),
                    desc: Joi.string().allow('').default(''),
                    defaultValue: Joi.when('style', {is: 'Input', then: Joi.string().allow('').default('')})
                        .when('style', {is: 'InputNumber', then: Joi.number().default(0)})
                        .when('style', {is: 'Select', then: Joi.string().allow('').default('')})
                        .when('style', {is: 'MultiSelect', then: Joi.array().items(Joi.string())})
                        .when('style', {is: 'Switch', then: Joi.boolean().default(false)})
                        .when('style', {is: 'Text', then: Joi.string().allow('').default('')})
                        .when('style', {is: 'DateTimePicker', then: Joi.date().iso()})
                        .when('style', {
                            is: 'DateTimePickerRange', then: Joi.object().keys({
                                min: Joi.date().iso(),
                                max: Joi.date().iso()
                            })
                        }),
                    minLength: Joi.number().integer().min(1).default(1),
                    maxLength: Joi.number().integer().min(1).default(1),
                    minValue:Joi.number().integer().default(0),
                    maxValue: Joi.number().integer().default(0),
                    minSelected: Joi.number().integer().min(1),
                    maxSelected:Joi.number().integer().min(1),
                    minDateTime:  Joi.date().iso().default(Date.now,"now"),
                    maxDateTime:  Joi.date().iso().default(Date.now,"now"),
                    storeValue: Joi.string().allow('').default(''),
                    selectionList:Joi.array().items({
                        label: Joi.string().required(),
                        value: Joi.string().required(),
                    }).default([]),
                }).unique((a, b) => a.name === b.name).min(1),
            }),
            query: Joi.object().keys({
                token: Joi.string().allow(''),
                targetFilename: Joi.string().regex(/^(\/[a-zA-Z0-9\-_]+)+$/).default('/.file-type'),
            }).rename('target_filename', 'targetFilename')
        }
    }
}
// controller/file-type.js
module.exports = app => {
    return class FileTypeController extends app.Controller {
        create() {
            if(this.operator.canCreateFileType(this.request.body.fileTypeName)){
                ...
            }
            if(this.operator.hasPermission(PM.CREATE_FILE_TYPE)){
                ...
            }
        }
    }
};

可能如果合并在controller中,那么controller的action会变得很膨胀,而且有重复的逻辑。

AnzerWall commented 7 years ago

@dead-horse 是,可能还要多次静态校验。 但是一般场景下只有从用户那边过来是危险区,我们这边一般是安全区,安全区去安全区是可以不必要校验。而且分离出一个总的前置校验和多不多次校验是不冲突,毕竟不是要去掉ctx.validate,而是加上了一个选择。

AnzerWall commented 7 years ago

@popomore 是这样的文件结构没错

atian25 commented 7 years ago

app/validator/*.js 自动加载这个问题不大, loader 提供了 API 来实现

// app.js
module.exports = app => {
  const schemeDir = path.join(app.config.baseDir, 'app/validator')
  app.loader.loadToApp(schemeDir, 'validator');
};

// app/validator/test.js
const Joi = require('joi');
module.exports = Joi => {
  return {};
};

// use
app.validator.xx
atian25 commented 7 years ago

然后自动校验的那个, 方法应该蛮多的, 譬如覆盖掉 app.get 等方法, 在里面加上判断 validator 是否存在的逻辑. 这块 @popomore 看有没有更好的建议.

https://github.com/eggjs/egg-core/blob/master/lib/utils/router.js#L72

AnzerWall commented 7 years ago

@atian25 我没有非常仔细看完框架,暂时不知道怎么覆盖,怎么在插件的条件下怎么覆盖掉这些方法呢。。。

dead-horse commented 7 years ago

@AnzerWall

在插件中:

// app/extend/application.js

module.exports = {
  getWithValidate(...args) {
    const action = args.pop();
    args = args.concat([ validate(action), action ]);
    return this.get(...args);
  },
}

应用中:

// app/router.js
module.exports = app => {
  app.getWithValidate('/users', 'user.list');
};
AnzerWall commented 7 years ago

@dead-horse 啊,对哦,我可以覆盖app,倒是没想到,一直想着覆盖router的get方法

atian25 commented 7 years ago

@AnzerWall 这个还有其他问题没?

没有的话可以 close 了.

BTW: 有时间可以写个经验总结.