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

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

Open
henryjhenry opened this issue Sep 27, 2024 · 3 comments
Labels
question Further information is requested

Comments

@henryjhenry
Copy link

目前引用了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的单测变得简单

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

@henryjhenry henryjhenry added the question Further information is requested label Sep 27, 2024
@kratos-ci-bot kratos-ci-bot changed the title [Question] parameters validation best practice (关于参数校验的最佳实践) [Question] parameters validation best practice (best practice about parameter validation) Sep 27, 2024
@kratos-ci-bot
Copy link
Collaborator

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
Copy link
Contributor

czyt commented Nov 15, 2024

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

  • Service 层:协议转换,比如grpc转http 和一些简单的validate。
  • Biz层:具体的Biz业务,跟协议无关。

@kratos-ci-bot
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants