elevator(updateFloors): walkthrough §3.5; guard task null and CloudwalkResult per §2.2

Made-with: Cursor
2026-06-09 08:20:31 +08:00 · 2026-04-25 08:24:24 +08:00
parent fdb2c04f11
commit 0ddeedc281
3 changed files with 139 additions and 11 deletions
@@ -10,7 +10,7 @@

 | 约定 § | 代码锚点 | 子任务数（上界） | 下一可修正动作（建议顺序） | 不可修正或须前置确认 |
 |--------|----------|------------------|----------------------------|------------------------|
-| **§3.1** | `ImageRuleRefServiceImpl#delete`（约 575～598 行） | **`N = param.getIds().size()`** 次 `updateGroupPersonRef`（每规则删后各 1 次） | **P0**：全部 `deleteById` 完成后，对本轮涉及的 `labelIds`、`organizationIds` **去重并集**，调用 **1 次** `updateGroupPersonRef`；为每次 RPC 增加 **`isSuccess` 校验**（与 §2.2 一致） | **须图库/通行确认**：合并调用是否为「刷新引用」语义、是否等价于当前 N 次效果；若不能确认则**不得合并**，仅可补返回值校验与日志 |
+| **§3.1** | `ImageRuleRefServiceImpl#delete`（约 575～598 行） | **`N = param.getIds().size()`** 次 `updateGroupPersonRef`（每规则删后各 1 次） | **P0**：全部 `deleteById` 完成后，对本轮涉及的 `labelIds`、`organizationIds` **去重并集**，调用 **1 次** `updateGroupPersonRef`；为每次 RPC 增加 **`isSuccess` 校验**（与 §2.2 一致） | **须图库/通行确认**：合并调用是否为「刷新引用」语义、是否等价于当前 N 次效果；若不能确认则**不得合并**，仅可补返回值校验与日志（**工作区走查与方案审核见 §6**） |
 | **§3.2** | `AcsPersonServiceImpl#delete`（约 165～175 行） | **`P = param.getPersonIds().size()`** 次 `imageStorePersonService.delete` | **P1**：**有界并行**（并发度 4～8）调用 `delete`，失败聚合与现网「遇错即停」一致；已具备 `isSuccess` 分支，保持语义 | **无批量 delete**：无法改为单次 RPC，除非将来扩展契约（约定 §4 远期） |
 | **§3.3** | `AcsPassRuleServiceImpl#listFloor` | **`F = passRuleResults.size()`** 次 `acsPersonService.page`（仅取 `totalRows`） | **首轮已完成**：`page.isSuccess()`、`getData()` 空防护、`rowsOfPage=1`。**P1 后续**：按楼层 **有界并行** `page`（保持结果顺序） | **禁止**用本地 `countPersonIdByZoneId` 等替代 `page.totalRows`；**无多 zone 一次统计 API** 时无法单 RPC 消除 N |
 | **§3.4** | `AcsPassRuleServiceImpl#addImageStore`（约 195～201 行） | **`D = deviceList.size()`** 次 `bindDeviceAndImageStore` | **P1**：**有界并行** `bind`；并行失败时与现有 **回滚删图库** 逻辑一致（注意竞态与顺序） | **无批量 bind**：不能合并为 1 次 RPC（契约不扩展时） |
@@ -30,11 +30,33 @@

 ---

-## 3. 建议下一迭代（不改 HTTP/Feign 签名）
+## 3. 迭代规划与下一迭代范围（不改 HTTP/Feign 签名）

-1. ~~**小改快赢**~~：`AcsPassRuleServiceImpl#listFloor` 已增加 **`page` 的 `isSuccess` 与空数据防护**、`rowsOfPage=1`（见 **§5**）。  
-2. **P0 阻塞项**：与图库团队确认 **`updateGroupPersonRef`** 在「多 label/org 一次传入」下的语义 → 通过后实施 **§3.1 合并**。  
-3. **P1 工程项**：为 §3.2 / §3.3（并行 `page`）/ §3.4 引入**统一有界线程池**（或复用现有异步池策略），并在 PR 中写明失败策略与超时。
+### 冻结范围（2026-04-25）
+
+| 项 | 说明 |
+|----|------|
+| **约定 §3.1 全范围** | 未取得图库/通行对 `updateGroupPersonRef` 语义确认前，**不进行**与该约定相关的**任何**代码修正与优化（含 **合并 N→1** 及 §6.4 所述**仅 `isSuccess` 校验**小步），避免在无确认期分散实现与回滚成本。 |
+| **恢复条件** | 图库书面或接口说明确认 + 在 [约定文档 §3.1](对外接口不变-远程调用与性能优化约定.md) 文末回填对接人、日期；再按 §1 表拆分 PR（合并与返回值校验可分步）。 |
+
+### 迭代 3（**§3.5 `updateFloors`**）
+
+| 字段 | 内容 |
+|------|------|
+| **状态** | **走查与首轮修正已完成**（见 **§7**）：`getById` 空防护、步骤级 `CloudwalkResult` 校验、`keepAliveSeconds` 绑定线程池。 |
+| **约定锚点** | **§3.5** `AcsDeviceTaskServiceImpl#updateFloors` |
+| **暂缓项** | 楼层有界并行、`AbortPolicy` 与 `catch` 语义、删楼 `ruleMap` 缺键等见 **§7.3**。 |
+
+### 迭代 4 及以后（P1 工程批次）
+
+在迭代 3 首轮修正稳定后，按 ROI 与风险分批推进（均需单独走查闸门）：
+
+1. **§3.2**：`AcsPersonServiceImpl#delete` — 有界并行 `imageStorePersonService.delete`，失败策略与现网一致。  
+2. **§3.3**：`AcsPassRuleServiceImpl#listFloor` — 有界并行 `acsPersonService.page`，**保持楼层顺序**合并结果。  
+3. **§3.4**：`AcsPassRuleServiceImpl#addImageStore` — 有界并行 `bindDeviceAndImageStore`，失败与**回滚删图库**与现逻辑一致。  
+4. **统一线程池**：迭代 3 若已梳理 `updateFloorsExecutor`，再决定 P1 是否复用命名池或独立有界池，并在 PR 中写明并发度、超时与遇错策略。
+
+**已完成回顾**：迭代 1 — `listFloor` 首轮修正（**§5**）；迭代 2 — §3.1 仅文档走查（**§6**），代码冻结见上表。

 ---

@@ -67,6 +89,84 @@

 ---

-## 6. 迭代 2 起（占位）
+## 6. 迭代 2：`ImageRuleRefServiceImpl#delete` 与 §3.1 方案走查（仅评估，未改代码）

- **§3.1** 待图库确认后再改代码；确认前不在此文档标记为「已实施」。
+**走查日期**：2026-04-25  
+**目标**：在全工作区定位**图库（intelligent 图库人员服务）**与**通行规则（电梯应用本地）**相关代码，审核「合并 `updateGroupPersonRef`」方案是否需图库侧语义确认后方可实施。
+
+### 6.1 图库 / 通行相关代码位置（工作区）
+
+| 层级 | 路径/符号 | 职责 |
+|------|-----------|------|
+| **Feign 契约** | `maven-intelligent-cwoscomponent/intelligent-cwoscomponent-rest/.../feign/ImageStorePersonFeignClient.java` | `POST .../updateGroupPersonRef`，请求体 `UpdateGroupPersonRefParam` |
+| **DTO** | `maven-intelligent-cwoscomponent/intelligent-cwoscomponent-interface/.../param/UpdateGroupPersonRefParam.java` | `businessId`、`imageStoreId`、`personIds`、`labelIds`、`organizationIds`（可同时只填部分字段） |
+| **客户端封装** | `.../service/RestImageStorePersonServiceImpl.java` | 透传 Feign |
+| **电梯侧 §3.1 锚点** | `maven-cw-elevator-application/.../passrule/impl/ImageRuleRefServiceImpl.java` 方法 **`delete`**（约 571～605 行） | 按 `param.getIds()` 循环：`listByParentRule` → 收集子规则 `includeLabels` / `includeOrganizations` → **`deleteById`** → **`imageStorePersonService.updateGroupPersonRef`**（每删一条父规则 1 次 RPC） |
+| **同文件其它 `updateGroupPersonRef`** | `addOnlyRule`（约 434～439）、`update` 内分支（约 556～562） | 新增/编辑规则后刷新；**同样未校验** `CloudwalkResult.isSuccess()` |
+| **人员通行规则** | `maven-cw-elevator-application/.../person/impl/PersonRuleServiceImpl.java` | `add` / `addVisitor` / `delete` 末尾各 1 次 `updateGroupPersonRef`，入参以 **`personIds`** 为主（与 `delete` 以 **label/org** 为主不同） |
+| **HTTP 入口** | `cw-elevator-application-web/.../AcsPassRuleController.java` | `imageRuleRefService.delete` |
+| **异步任务调用** | `AcsDeviceTaskServiceImpl#updateFloors` | 删楼层时多构造 `deleteParam.setIds(Collections.singletonList(ruleId))` **逐层**调 `imageRuleRefService.delete`；单次 `delete` 内仍可能 1 次或多次 `updateGroupPersonRef`（视该层 `ids` 数量） |
+
+**说明**：本仓库**无** intelligent 图库服务端的 `updateGroupPersonRef` 业务实现源码，仅能依据 DTO 与调用方推断语义；**与图库/通行团队确认**仍属 §3.1 前置条件。
+
+### 6.2 当前 `delete` 行为摘要（与合并相关）
+
+- `imageStoreId` 来自 **`deviceImageStoreDao.getByBuildingId(param.getParentId())`**，整次批量删除共用同一图库。
+- 对每个待删父规则 `id`：先读**该父规则下子规则**的 label/org（子行若带 label 则 `continue`，**同一子行不会同时写入 org**，与数据模型一致），再删父规则，再带着**本轮** `includeLabels` / `includeOrganizations` 调图库刷新。
+- **`updateGroupPersonRef` 的返回值未做 `isSuccess` 校验**（违反约定 §2.2；与 `getImageStorePerson` 等分支不一致）。
+
+### 6.3 约定中的「合并」方案审核
+
+| 维度 | 结论 |
+|------|------|
+| **与现网 N 次调用的等价性** | 若图库侧语义为：在**给定 `imageStoreId`** 下，对传入的 **labelIds / organizationIds 集合**做**增量刷新或按维度重算引用**（各维度独立、与顺序无关），则「删库前汇总所有待删父规则的子维度 → **去重并集** → 删库完成后 **1 次** `updateGroupPersonRef`」与「每删一条父规则刷新其维度子集」在**最终一致**上通常等价。 |
+| **必须向图库确认的风险** | 若远端实现为「以本次入参**覆盖/裁剪**图库可见范围」或依赖**调用顺序**产生副作用，则合并后的**单次并集**与 N 次**子集递进**可能不等价。约定文档 §3.1 所述「非破坏性刷新」即针对此。 |
+| **`personIds` 与 `labelIds`/`organizationIds` 混用** | `PersonRuleServiceImpl` 走 `personIds` 路径；`ImageRuleRefServiceImpl#delete` 走 label/org。合并方案**不改变** `delete` 仅设 label/org 的现状；但若图库服务在**未传 `personIds`** 时对空列表有特殊含义，仍须一并确认。 |
+| **空列表仍 RPC** | 当前循环在子规则为空时仍调用 `updateGroupPersonRef`（两列表皆空）。合并后是否**跳过空并集**可减少无效 RPC，但属于**行为微调**，若图库依赖「空刷」触发全量重算，须图库确认后再定。 |
+| **不合并时的安全增量** | 在未获图库书面确认前，**仅**可为每次 `updateGroupPersonRef` 增加 **`isSuccess` 校验 + 失败抛 `ServiceException`**（及可选日志），**不改变** RPC 次数；与约定「退化为循环调用 + 返回值校验」一致。 |
+
+### 6.4 评估结论（是否允许进入 §3.1「合并」类代码修正）
+
+- **合并 N→1**：**不允许在图库/通行确认前实施** — 与 §1 表及约定 §3.1 前置条件一致。  
+- **仅返回值校验（及可选空并集跳过，若产品同意）**：**允许作为独立小步** — 不依赖远端语义新假设，符合 §2.2。
+
+**图库确认建议提问（可复制）**：「对同一 `imageStoreId`，`updateGroupPersonRef` 在仅设置 `labelIds`、`organizationIds`（`personIds` 为空）时，是否为**按这些维度刷新人员引用**且**不会**将图库维度裁剪为仅等于本次入参？多次调用子集与单次调用**并集**是否在业务上等价？」
+
+**实施后回填**：确认结论、对接人、日期写入 [约定文档 §3.1 文末](对外接口不变-远程调用与性能优化约定.md)（见约定 §5）。
+
+**排期决策（2026-04-25）**：在取得图库确认前，**冻结**约定 **§3.1** 相关全部代码变更；下一迭代转 **§3.5**（见上文 **「迭代 3」**）。
+
+---
+
+## 7. 迭代 3：`AcsDeviceTaskServiceImpl#updateFloors` 走查结论（§3.5）
+
+**走查日期**：2026-04-25  
+**代码位置**：`cw-elevator-application-service/.../device/impl/AcsDeviceTaskServiceImpl.java` 方法 `updateFloors`；线程池 `.../common/UpdateFloorsTaskExecutor.java`；配置 `UpdateFloorsPoolProperties`（`ninca.update.floor.pool.*`，默认 core=3、max=5、queue=100、`AbortPolicy`）。
+
+### 7.1 调用链与 RPC 上界（与 §1 表对齐）
+
+| 分支 | 行为 | 上界 |
+|------|------|------|
+| 增楼层 | `personRuleService.add` **或** `imageRuleRefService.addOnlyRule`，成功后 `updateBingDevices` | `addFloors.size()` |
+| 删楼层 | `personRuleService.delete` **或** `imageRuleRefService.delete`（单 id）**或** 仅 DAO `deleteByOrgAndLabel`，成功后 `updateBingDevices` | `delFloorIds.size()` |
+| 内层放大 | `imageRuleRefService.delete` 仍受约定 **§3.1** 冻结影响（`updateGroupPersonRef` 多次）；本迭代**未**改该内层。 | 不变 |
+
+**入口**：`AcsElevatorDeviceServiceImpl#bindingFloors` / `#bindingPerson` 在插入任务行后**同步**调用 `updateFloors`；方法带 `@Async`，实际在 **`updateFloorsExecutor`** 线程执行；HTTP 已返回 `taskId` 后，**异步内失败不会回写该 HTTP 响应**（现网行为保持；运维依赖任务进度与日志）。
+
+### 7.2 检查项与结论
+
+| 检查项 | 现状（走查时） | 结论 |
+|--------|----------------|------|
+| `acsDeviceTaskDao.getById` | 未判空即 `task.getIsStop()`，存在 **NPE** 风险（数据异常或竞态） | **不通过** → 已修正：空则记录并 `ServiceException` |
+| `personRuleService.add/delete`、`imageRuleRefService.addOnlyRule/delete` 返回值 | 未校验 `CloudwalkResult.isSuccess()`，失败时仍 **`updateBingDevices`**，进度与真实绑定不一致 | **不通过**（违反 §2.2）→ 已修正：统一 `requireTaskStepSuccess`，失败抛错且**不**递增 |
+| `catch` 仅 `ServiceException(e.getMessage())` | 丢失根因类型与栈信息到调用方；异步场景仅日志含 `{}` 与异常 | **记录**：是否改为 `ServiceException(code, msg)` 或 `initCause` 属产品/运维范围，**本轮不改** |
+| 线程池 `keepAliveSeconds` | `UpdateFloorsPoolProperties` 有字段，**Bean 未 `setKeepAliveSeconds`**，配置项无效 | **缺陷** → 已在 `UpdateFloorsTaskExecutor` 绑定 |
+| `RejectedExecutionHandler` | `AbortPolicy`，队列满时拒绝提交 | **记录**：与背压策略相关，**本轮不改**（须与运维对齐） |
+| 删楼 `ruleMap.get(delFloorId)` | 若 `listZoneInfoByIds` 未覆盖某 `delFloorId` 可能 **null** 拼接 `ruleName` | **记录**：数据正常时风险低；**本轮未改**（可后续与 DAO 对齐） |
+
+### 7.3 评估结论（是否允许进入代码修正）
+
+- **允许并已实施（本轮）**：`task` 空指针防护；对 **`personRuleService.add` / `delete`**、**`imageRuleRefService.addOnlyRule` / `delete`** 的 **`CloudwalkResult` 成功校验**（约定 §2.2）；`updateFloorsExecutor` 绑定 **`keepAliveSeconds`**。  
+- **暂缓（须单独评审）**：按楼层 **有界并行**、拒绝策略、`catch` 异常语义增强、`ruleMap` 缺键防护。
+
+**修正实施后**：提交 **`e88ed9b`**（分支 `v0.11`）。
@@ -18,6 +18,7 @@ public class UpdateFloorsTaskExecutor {
        threadPoolTaskExecutor.setAllowCoreThreadTimeOut(this.updateFloorsPoolProperties.isAllowCoreThreadTimeOut());
        threadPoolTaskExecutor.setMaxPoolSize(this.updateFloorsPoolProperties.getMaxPoolSize());
        threadPoolTaskExecutor.setQueueCapacity(this.updateFloorsPoolProperties.getQueueCapacity());
+        threadPoolTaskExecutor.setKeepAliveSeconds(this.updateFloorsPoolProperties.getKeepAliveSeconds());
        threadPoolTaskExecutor.setThreadNamePrefix("update-floors-pool-");
        threadPoolTaskExecutor.setRejectedExecutionHandler(new ThreadPoolExecutor.AbortPolicy());
        threadPoolTaskExecutor.initialize();
@@ -2,6 +2,7 @@ package cn.cloudwalk.elevator.device.impl;

 import cn.cloudwalk.cloud.context.CloudwalkCallContext;
 import cn.cloudwalk.cloud.exception.ServiceException;
+import cn.cloudwalk.cloud.result.CloudwalkResult;
 import cn.cloudwalk.elevator.common.AbstractAcsDeviceService;
 import cn.cloudwalk.elevator.device.dao.AcsDeviceTaskDao;
 import cn.cloudwalk.elevator.device.dto.AcsDeviceTaskAddDto;
@@ -46,6 +47,10 @@ public class AcsDeviceTaskServiceImpl extends AbstractAcsDeviceService implement
            if (!CollectionUtils.isEmpty(addFloors)) {
                for (AcsPassRuleImageResultDto addFloor : addFloors) {
                    AcsDeviceTaskDTO task = this.acsDeviceTaskDao.getById(param.getTaskId());
+                    if (task == null) {
+                        this.logger.error("updateFloors 任务不存在 taskId={}", param.getTaskId());
+                        throw new ServiceException("设备任务不存在");
+                    }
                    if (task.getIsStop().intValue() == 0) {
                        if (!ObjectUtils.isEmpty(param.getPersonId())) {
                            AcsPersonAddParam addParam = new AcsPersonAddParam();
@@ -53,7 +58,8 @@ public class AcsDeviceTaskServiceImpl extends AbstractAcsDeviceService implement
                            addParam.setParentId(param.getParentId());
                            addParam.setZoneId(addFloor.getZoneId());
                            addParam.setZoneName(addFloor.getZoneName());
-                            this.personRuleService.add(addParam, context);
+                            CloudwalkResult<Boolean> addResult = this.personRuleService.add(addParam, context);
+                            requireTaskStepSuccess(addResult, "personRuleService.add");
                        } else {
                            AcsPassRuleNewParam ruleParam = new AcsPassRuleNewParam();
                            ruleParam.setParentId(param.getParentId());
@@ -67,7 +73,9 @@ public class AcsDeviceTaskServiceImpl extends AbstractAcsDeviceService implement
                                ruleParam.setIncludeOrganizations(Collections.singletonList(param.getOrgId()));
                                ruleParam.setRuleName(addFloor.getZoneName() + param.getOrgName());
                            }
+                            CloudwalkResult<Boolean> addRuleResult =
                                this.imageRuleRefService.addOnlyRule(ruleParam, context);
+                            requireTaskStepSuccess(addRuleResult, "imageRuleRefService.addOnlyRule");
                        }
                        AcsDeviceTaskAddDto addDto = new AcsDeviceTaskAddDto();
                        addDto.setId(task.getId());
@@ -82,13 +90,18 @@ public class AcsDeviceTaskServiceImpl extends AbstractAcsDeviceService implement
                ruleList.forEach(rule -> ruleMap.put(rule.getZoneId(), rule.getZoneName()));
                for (String delFloorId : delFloorIds) {
                    AcsDeviceTaskDTO task = this.acsDeviceTaskDao.getById(param.getTaskId());
+                    if (task == null) {
+                        this.logger.error("updateFloors 任务不存在 taskId={}", param.getTaskId());
+                        throw new ServiceException("设备任务不存在");
+                    }
                    if (task.getIsStop().intValue() == 0) {
                        if (!ObjectUtils.isEmpty(param.getPersonId())) {
                            AcsPersonDeleteParam delParam = new AcsPersonDeleteParam();
                            delParam.setParentId(param.getParentId());
                            delParam.setZoneId(delFloorId);
                            delParam.setPersonIds(Collections.singletonList(param.getPersonId()));
-                            this.personRuleService.delete(delParam, context);
+                            CloudwalkResult<Boolean> delResult = this.personRuleService.delete(delParam, context);
+                            requireTaskStepSuccess(delResult, "personRuleService.delete");
                        } else {
                            String ruleName = "";
                            if (!ObjectUtils.isEmpty(param.getLabelName())) {
@@ -103,7 +116,9 @@ public class AcsDeviceTaskServiceImpl extends AbstractAcsDeviceService implement
                                deleteParam.setIds(Collections.singletonList(ruleId));
                                deleteParam.setZoneId(delFloorId);
                                deleteParam.setParentId(param.getParentId());
+                                CloudwalkResult<Boolean> delRuleResult =
                                    this.imageRuleRefService.delete(deleteParam, context);
+                                requireTaskStepSuccess(delRuleResult, "imageRuleRefService.delete");
                            } else {
                                AcsPassRuleDeleteDto dto = new AcsPassRuleDeleteDto();
                                dto.setZoneId(delFloorId);
@@ -124,4 +139,16 @@ public class AcsDeviceTaskServiceImpl extends AbstractAcsDeviceService implement
            throw new ServiceException(e.getMessage());
        }
    }
+
+    /**
+     * 约定 §2.2：异步任务内对业务服务返回的 {@link CloudwalkResult} 须校验成功后再推进进度（避免失败仍递增 bindDevices）。
+     */
+    private void requireTaskStepSuccess(CloudwalkResult<?> result, String op) throws ServiceException {
+        if (result == null || !result.isSuccess()) {
+            String code = result != null ? result.getCode() : "76260540";
+            String msg = result != null ? result.getMessage() : op + " 返回为空";
+            this.logger.error("updateFloors 步骤失败 op={} code={} msg={}", op, code, msg);
+            throw new ServiceException(code, msg);
+        }
+    }
 }