go-kratos / kratos

Your ultimate Go microservices framework for the cloud-native era.
https://go-kratos.dev
MIT License
23.38k stars 4.01k forks source link

[Question] parameters validation best practice (best practice about parameter validation) #3430

Open henryjhenry opened 1 month ago

henryjhenry commented 1 month ago

目前引用了PGV(proto-gen-validate),对于一些静态类型和数据限制的校验很方便,但是遇到一些需要查询库才能做校验的逻辑,应该放到哪一层比较合适呢?

比如:调用方传了一个用户ID过来,我需要先查库确定这个用户是否存在,如果用户存在的话,会查询出一个 User DOUser DO再贯穿整个usecase逻辑,但是现在遇到两个问题:

  1. 如果把校验逻辑放在usecase层,那么有两个方案: a. 在usecase逻辑中做参数校验:

        func(u *Usecase) Logic(params Params) error {
            userDO, err := userRepo.RetrieveByID(params.UserID)
            if err != nil {
                return err
            }
    
            // ...
        }

    这样会有一个问题,userDO是在Logic中新创建的,会导致在单测时不得不mock掉userRepo, 鉴于go的mock不是特别好用,我觉得有点麻烦

    b. 在usecase加一个RetrieveUser方法,把检验逻辑前置到service层:

    func (s *Service) Handle(req *Request) error {
        user, err := s.usecase.RetrieveByID(req.UserID)
        if err != nil {
            return err
        }
        params := biz.Params{
            User: user,
        }
        s.usercase.Logic(params)
    }

    这样做倒是简化了Logic的单测逻辑, 只需要构造Params即可,但是usecase会变得很膨胀

  2. 把检验放到service层: 参考1.b,在service层依赖userRepo:

    type Service struct {
        userRepo biz.UserRepo
    }
    
    func (s *Service) Handle(req *Request) error {
        user, err := s.userRepo.RetrieveUserByID(req.UserID)
        if err != nil {
            return err
        }
        params := biz.Params{
            User: user,
        }
        s.usecase.Logic(params)
    }

    我个人是比较倾向于这种方案,这样做保证了几点:serviceusecase层依赖的是同一个userRepo,不会产生逻辑偏差;usecase.Logic的单测变得简单

想问下大家还有没有更好的实现方案

kratos-ci-bot commented 1 month ago

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Currently, PGV (proto-gen-validate) is quoted, which is very convenient for verifying some static types and data restrictions. However, when encountering some logic that requires querying the library for verification, which layer should it be placed at?

For example: the caller passes a user ID, and I need to check the database first to determine whether the user exists. If the user exists, a User DO will be queried, and User DO will run through the entire usecase logic, but now Encountered two problems:

  1. If you put the verification logic in the usecase layer, there are two options: a. Do parameter verification in usecase logic:

        func(u *Usecase) Logic(params Params) error {
            userDO, err := userRepo.RetrieveByID(params.UserID)
            if err != nil {
                return err
            }
    
            // ...
        }

    There will be a problem. userDO is newly created in Logic, which will cause userRepo to be mocked during single testing. Considering that go’s mock is not particularly easy to use, I think it is a bit troublesome.

    b. Add a RetrieveUser method to usecase and forward the verification logic to the service layer:

    func (s *Service) Handle(req *Request) error {
        user, err := s.usecase.RetrieveByID(req.UserID)
        if err != nil {
            return err
        }
        params := biz.Params{
            User: user,
        }
        s.usercase.Logic(params)
    }

    This simplifies the single test logic of Logic. You only need to construct Params, but usecase will become very bloated.

  2. Put the inspection in the service layer: Refer to 1.b, which depends on userRepo in the service layer:

    type Service struct {
        userRepo biz.UserRepo
    }
    
    func (s *Service) Handle(req *Request) error {
        user, err := s.userRepo.RetrieveUserByID(req.UserID)
        if err != nil {
            return err
        }
        params := biz.Params{
            User: user,
        }
        s.usecase.Logic(params)
    }

    I personally prefer this solution. This ensures several points: the service and usecase layers rely on the same userRepo, which will not cause logical deviation; the single test of usecase.Logic becomes Simple

I would like to ask if you have any better implementation plan

czyt commented 1 day ago

这个问题我以前问过,实际上就是service层的事情,不应该放在biz层去。以前问毛大这个问题,回复如下:

kratos-ci-bot commented 1 day ago

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


I have asked this question before. It is actually a matter of the service layer and should not be placed on the biz layer. I asked Mao Da this question before, and his reply was as follows:

  • Service layer: protocol conversion, such as grpc to http and some simple validate.
  • Biz layer: The specific Biz business has nothing to do with the protocol.