Skip to content

Conversation

@limit81995
Copy link
Contributor

例如使用validation功能的以下场景时:

    protected array $scenes = [
        "detail" => [
            "phone" => ["regex:/^1[345789]\d{9}$/"],
            "username",
            "email",
        ],
    ];


    public function rules(): array
    {
        return [
            "phone" => ["required", "regex:/^1[345789]\d{9}$/"],
        ];
    }

之前版本是会报错。如果删除$scenes["detail"]["phone"]的赋值后,会正常使用,但是场景只能引用下面定义的phone规则。

有时会遇到,A接口和B接口都需要用phone,但是A接口phone是必填,B接口phone是非必填,但是如果填写的话就需要走phone的规则。 如果用其他方式实现会比较复杂,例如创建一个新的验证器,那么message和attributes的方法都要复制过去。再或者写很多方法去进行手动验证,这样极不方便解耦和提高效率。

所以如果在scenes中可以直接将场景的使用的参数进行覆盖就可以大大减少很多工作量。

@huangdijia
Copy link
Member

代码评审报告

概述

此PR为Hyperf的验证器功能增加了在场景(scene)中直接定义规则的能力,允许覆盖默认规则。这解决了多接口使用相同字段但验证规则不同的问题。

代码质量分析

正面方面:

  • 解决了实际的业务痛点,提高了验证器的灵活性
  • 代码逻辑清晰,实现简洁
  • 保持了向后兼容性

需要改进的方面:

1. 代码质量问题

  • 变量命名不够清晰: $rule 在不同上下文中表示不同含义,容易混淆
  • 逻辑可读性: 当前的 if-elseif 结构不够直观

2. 潜在的功能问题

  • 键冲突处理: 当场景中既有字符串引用又有数组定义同一个字段时,行为不明确
  • 错误处理: 对无效的场景配置缺乏验证和错误提示

3. 性能考虑

  • 每次调用都会遍历所有场景规则,对于大型场景可能影响性能

具体改进建议

protected function getRules(): array
{
    $rules = $this->rules();
    $scene = $this->getScene();
    
    if (\!$scene || \!isset($this->scenes[$scene]) || \!is_array($this->scenes[$scene])) {
        return $rules;
    }
    
    $sceneRules = [];
    $availableRuleKeys = array_keys($rules);
    
    foreach ($this->scenes[$scene] as $fieldName => $ruleDefinition) {
        // 如果是数组形式(field => rules),直接覆盖规则
        if (is_string($fieldName) && is_array($ruleDefinition)) {
            $sceneRules[$fieldName] = $ruleDefinition;
        }
        // 如果是字符串形式(引用现有规则)
        elseif (is_numeric($fieldName) && is_string($ruleDefinition) && in_array($ruleDefinition, $availableRuleKeys)) {
            $sceneRules[$ruleDefinition] = $rules[$ruleDefinition];
        }
    }
    
    return $sceneRules;
}

测试覆盖率

  • 缺少单元测试: PR中没有包含对新功能的测试用例
  • 建议添加测试: 需要测试场景规则覆盖、混合使用等场景

安全考虑

  • 当前实现基本安全,但建议添加对场景配置的验证以防止配置错误

总体评价

这是一个有价值的功能增强,解决了实际问题。但需要:

  1. 改进代码可读性和变量命名
  2. 添加完整的单元测试
  3. 考虑边界情况的处理
  4. 更新相关文档

建议: 在合并前完善测试用例和代码优化。

🤖 Generated with Claude Code

@limingxinleo limingxinleo changed the title feat:增加支持在scene中定义rule用来覆盖默认rule,方便在使用多接口时,只用定义一个文件切换场景使用。 Support to rewrite the rules for scene when using validation. Sep 25, 2025
limingxinleo
limingxinleo previously approved these changes Sep 25, 2025
@limingxinleo limingxinleo merged commit 11ee035 into hyperf:master Sep 25, 2025
71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants