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

Made-with: Cursor

Former-commit-id: 0ddeedc281
This commit is contained in:
反编译工作区
2026-04-25 08:24:24 +08:00
parent b43a38358e
commit cce994c043
3 changed files with 139 additions and 11 deletions
@@ -10,7 +10,7 @@
| 约定 § | 代码锚点 | 子任务数(上界) | 下一可修正动作(建议顺序) | 不可修正或须前置确认 |
|--------|----------|------------------|----------------------------|------------------------|
| **§3.1** | `ImageRuleRefServiceImpl#delete`(约 575598 行) | **`N = param.getIds().size()`** 次 `updateGroupPersonRef`(每规则删后各 1 次) | **P0**:全部 `deleteById` 完成后,对本轮涉及的 `labelIds``organizationIds` **去重并集**,调用 **1 次** `updateGroupPersonRef`;为每次 RPC 增加 **`isSuccess` 校验**(与 §2.2 一致) | **须图库/通行确认**:合并调用是否为「刷新引用」语义、是否等价于当前 N 次效果;若不能确认则**不得合并**,仅可补返回值校验与日志 |
| **§3.1** | `ImageRuleRefServiceImpl#delete`(约 575598 行) | **`N = param.getIds().size()`** 次 `updateGroupPersonRef`(每规则删后各 1 次) | **P0**:全部 `deleteById` 完成后,对本轮涉及的 `labelIds``organizationIds` **去重并集**,调用 **1 次** `updateGroupPersonRef`;为每次 RPC 增加 **`isSuccess` 校验**(与 §2.2 一致) | **须图库/通行确认**:合并调用是否为「刷新引用」语义、是否等价于当前 N 次效果;若不能确认则**不得合并**,仅可补返回值校验与日志(**工作区走查与方案审核见 §6**) |
| **§3.2** | `AcsPersonServiceImpl#delete`(约 165175 行) | **`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`(约 195201 行) | **`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`**(约 571605 行) | 按 `param.getIds()` 循环:`listByParentRule` → 收集子规则 `includeLabels` / `includeOrganizations`**`deleteById`** → **`imageStorePersonService.updateGroupPersonRef`**(每删一条父规则 1 次 RPC) |
| **同文件其它 `updateGroupPersonRef`** | `addOnlyRule`(约 434439)、`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());
}
this.imageRuleRefService.addOnlyRule(ruleParam, context);
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());
this.imageRuleRefService.delete(deleteParam, context);
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);
}
}
}