<fix>[storage]: ZSTAC-80789 return defensive copy from getPreferBackupStorageTypes

[zstack-robot-2]

问题

ExternalPrimaryStorage.handle(SelectBackupStorageMsg) 中 selector.getPreferBackupStorageTypes() 直接返回 Spring Bean 内部字段引用，后续 retainAll() 原地修改了 Bean 状态，导致一次触发后所有后续克隆操作的备份存储类型过滤永久失效。

修复

在 XInfiniStorageFactory、ExponStorageFactory、ZbsStorageFactory 三个实现中，getPreferBackupStorageTypes() 返回 new ArrayList<>(preferBackupStorageTypes) 防御性拷贝。

测试

• ExternalPrimaryStorageSelectBsTest 2 个 UT 验证修复有效且能复现原始 bug

Resolves: ZSTAC-80789

sync from gitlab !9406

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: 2e0f8e07-b161-46fb-9940-dd3216fd582c

📥 Commits

Reviewing files that changed from the base of the PR and between 9d25b73 and f612c77.

📒 Files selected for processing (1)

• test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/ExternalPrimaryStorageSelectBackupStorageCase.groovy

---

概述

三个存储工厂类（Expon、XInfini、Zbs）中的 getPreferBackupStorageTypes() 方法现在返回内部列表的防御性副本，防止外部调用者直接修改存储的列表。同时更新了相应的集成测试用例，以验证此防御复制行为。

变更

队列/文件	摘要
防御性复制实现 plugin/expon/src/main/java/org/zstack/expon/ExponStorageFactory.java, plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageFactory.java, plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageFactory.java	在 getPreferBackupStorageTypes() 方法中返回 new ArrayList<>(preferBackupStorageTypes) 而非直接返回内部列表，增加了 ArrayList 导入以支持此变更。
测试用例更新 test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/ExternalPrimaryStorageSelectBackupStorageCase.groovy	重写测试用例以验证 getPreferBackupStorageTypes() 的防御复制行为（ZSTAC-80789）。新增集群引用和虚拟机创建，附加备份存储类型，通过两次 SelectBackupStorageMsg 调用验证连续调用不会破坏 bean 的存储列表。

代码审查工作量估计

🎯 2 (Simple) | ⏱️ ~15 分钟

诗

兔子欢呼雀跃，小爪轻轻一点，
列表不再外泄，副本保护周全！
防御复制守卫，数据岁月静好，🐰
存储工厂安心，测试验证周详。

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
Title check	❌ Error	The title exceeds the 72-character limit at 82 characters and uses the correct format.	Shorten the title to 72 characters or less, for example: '[storage]: ZSTAC-80789 return defensive copy from getter'.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description clearly explains the problem, the fix applied to three factory classes, and references the test updates, all directly related to the changeset.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sync/jin.ma/fix/ZSTAC-80789-5.5.12

📝 Coding Plan

• Generate coding plan for human review comments

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageFactory.java (1)

58-60: 潜在的空指针异常风险

如果 preferBackupStorageTypes 在 setter 被调用前为 null，new ArrayList<>(null) 会抛出 NullPointerException。

考虑到这是一个 Spring Bean，setter 通常会在初始化时被调用，但建议添加防御性检查以增强健壮性：

🛡️ 建议添加空值检查

 `@Override`
 public List<String> getPreferBackupStorageTypes() {
+    if (preferBackupStorageTypes == null) {
+        return new ArrayList<>();
+    }
     return new ArrayList<>(preferBackupStorageTypes);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageFactory.java`
around lines 58 - 60, The getter getPreferBackupStorageTypes can throw NPE when
preferBackupStorageTypes is null; modify it to defensively handle null by
returning Collections.emptyList() or new ArrayList<>() when
preferBackupStorageTypes == null, otherwise return a new
ArrayList<>(preferBackupStorageTypes); ensure any setter or initialization code
for preferBackupStorageTypes remains unchanged but the getter never constructs
ArrayList from a null reference.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageFactory.java`:
- Around line 58-60: The getter getPreferBackupStorageTypes can throw NPE when
preferBackupStorageTypes is null; modify it to defensively handle null by
returning Collections.emptyList() or new ArrayList<>() when
preferBackupStorageTypes == null, otherwise return a new
ArrayList<>(preferBackupStorageTypes); ensure any setter or initialization code
for preferBackupStorageTypes remains unchanged but the getter never constructs
ArrayList from a null reference.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: 2e5e39da-3ef7-4aa5-8ee0-0bb75bc3147a

📥 Commits

Reviewing files that changed from the base of the PR and between b42842c and 9d25b73.

📒 Files selected for processing (4)

• plugin/expon/src/main/java/org/zstack/expon/ExponStorageFactory.java
• plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageFactory.java
• plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageFactory.java
• storage/src/test/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageSelectBsTest.java