PR 审查报告：#5565 - 机上portal优化和部分打点

仓库：OpenHarmony/communication_wifi
PR 地址：https://gitcode.com/openharmony/communication_wifi/merge_requests/5565
作者：Aki (daweq)
目标分支：master
PR SHA：21aba6af92d3eaf00aa39c074714e3df8ab6f949
状态：open
标签：waiting_on_author、dco检查成功

CI 状态：

• ✅ DCO 检查成功
• ✅ 冲突检查通过

---

变更概要

本 PR 在 STA 状态机"网络检查通过（WORKING）"路径中新增产品信息获取触发（1 个文件，+3/-0）：

void StaStateMachine::HandleNetCheckResultIsWorking(SystemNetWorkState netState, ...)
{
    WifiConfigCenter::GetInstance().SetWifiSelfcureResetEntered(false);
    SaveLinkstate(ConnState::CONNECTED, DetailedState::WORKING);
    InvokeOnStaConnChanged(OperateResState::CONNECT_NETWORK_ENABLED, linkedInfo);
+    if (enhanceService_ != nullptr) {
+        ShouldGetProductInfo();
+    }
    lastCheckNetState_ = OperateResState::CONNECT_NETWORK_ENABLED;
    ...
}

---

问题详情

⚠️主要

1. enhanceService_ != nullptr 守卫与 ShouldGetProductInfo() 的调用关系不成立

位置：wifi/services/wifi_standard/wifi_framework/wifi_manage/wifi_sta/sta_state_machine.cpp（HandleNetCheckResultIsWorking）

描述：

if (enhanceService_ != nullptr) {
    ShouldGetProductInfo();
}

ShouldGetProductInfo() 是 StaStateMachine 的成员函数，其执行并不直接依赖调用点的 enhanceService_ 判空——若函数内部使用 enhanceService_，正确防护应在函数内部（或该函数的实现路径）判空；若函数内部不使用 enhanceService_，则此守卫为无关条件，可能误导后续维护者（以为"增强服务存在时才获取产品信息"，实际可能并非如此）。

影响：防护语义不清晰；若 ShouldGetProductInfo 内部存在其他使用 enhanceService_ 的路径，此守卫无法覆盖。

修复建议：确认 ShouldGetProductInfo() 的实现：

• 若内部使用 enhanceService_ 且已判空 → 删除外层冗余守卫；
• 若内部使用但未判空 → 在内部补判空，外层守卫可保留但需注释说明：

// enhanceService 存在时才查询产品信息（机上场景），避免无增强服务时做无效调用
if (enhanceService_ != nullptr) {
    ShouldGetProductInfo();
}

---

2. ShouldGetProductInfo 命名与语义需确认（返回类型/场景过滤）

描述：函数名 ShouldGetProductInfo 形似"是否应获取产品信息"的判断函数（惯例返回 bool），但此处按语句使用（忽略返回值）。需确认：

• 返回类型是 void 还是 bool？若为 bool，此处忽略返回值是否遗漏逻辑分支；
• 内部是否包含机上场景过滤（如航空 WiFi / 特定 SSID / 增强服务能力判断）。

影响：若内部无场景过滤，则所有设备在每次网络变为 WORKING 时都会触发产品信息获取/打点——普通家庭/办公网络连接也会执行，产生非预期副作用（查询开销、打点噪音、隐私相关数据上报）。

修复建议：明确语义并确认场景过滤：

// 若为判断函数，应改为条件调用：
if (ShouldGetProductInfo()) {
    // 执行获取逻辑
}

并确认仅机上/航空 WiFi 场景才返回 true（或内部直接按场景过滤）。

---

3. 调用点位于高频路径，需评估触发频率

描述：HandleNetCheckResultIsWorking 在网络状态恢复 WORKING 时触发，可能伴随反复断连重连（如机上信号波动）多次进入。若 ShouldGetProductInfo 无去重/节流，将重复触发查询与打点。

影响：重复打点污染统计；重复查询增加负载。

修复建议：确认 ShouldGetProductInfo 内部有状态去重（如已获取过则跳过），否则建议在调用点节流。

---

💡建议

4. 变更说明缺失

PR 标题"机上portal优化和部分打点"未说明：

• 产品信息获取的用途（机上门户展示？）；
• 与 Portal 认证流程的关系（isPortal 场景下是否会触发，见该函数后续 if (getCurrentWifiDeviceConfig().isPortal) 分支）；
• 打点的具体事件与格式。

建议补充，便于评审与排障。

5. 无测试

建议补充覆盖：enhanceService_ 为空/非空两种场景下 HandleNetCheckResultIsWorking 的行为。

---

总体评分

维度	评分	说明
功能正确性	5/10	守卫与调用关系不清晰，语义待确认
代码质量	5/10	命名/场景过滤不明确
安全性	6/10	需确认产品信息获取的隐私边界
可维护性	5/10	说明缺失
测试充分性	4/10	无测试
综合评分	5/10

审查结论

建议操作：⚠️ 暂不合并（需作者澄清）

建议修复：

• 问题 1：澄清 enhanceService_ 守卫与 ShouldGetProductInfo 的真实依赖关系；
• 问题 2：确认函数返回类型与机上场景过滤（避免普通网络也触发）；
• 问题 3：确认/增加去重或节流。

优点：

• 变更小（1 文件，+3/-0），触发点位于网络连通确认处，时机合理
• 空指针防护意识存在
• DCO 检查通过