<fix>[compute]: first create of zone set default #2648
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough新增在创建 Zone 时的预检：若数据库中当前无 Zone（count==0），自动将首次创建的 Zone 标记为默认（isDefault=true）；同时将若干数据库导入改为通配符导入，并增加集成测试验证该行为。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as API Client
participant ZM as ZoneManagerImpl
participant DB as Database
Client->>ZM: APICreateZoneMsg
rect rgb(240,248,255)
note right of ZM: 预检：查询 Zone 数量
ZM->>DB: Q.New(ZoneVO).count()
DB-->>ZM: total = 0 或 >0
alt total == 0
ZM->>ZM: msg.setDefault(true)
else total > 0
ZM->>ZM: 按现有默认区逻辑处理
end
end
ZM->>DB: 创建 Zone（默认/非默认 分支）
DB-->>ZM: 返回 ZoneInventory
ZM-->>Client: 返回 ZoneInventory
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml) Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/src/test/groovy/org/zstack/test/integration/compute/ZoneCase.groovy (1)
237-245: 用例未自证“系统无 Zone”前置状态；请在用例内部清空并加强断言为消除对执行顺序/环境的依赖，建议在用例开始前删除所有现存 Zone，并补充数据库层面的断言，确保仅创建了一个默认区。
建议修改：
void testFirstZoneSetDefaultZone() { + // ensure empty state + List<ZoneInventory> exists = queryZone { + conditions = [] + } + exists.each { z -> + deleteZone { + uuid = z.uuid + } + } + ZoneInventory zone = createZone { name = "FirstZone" description = "first zone" } as ZoneInventory assert zone != null assert zone.isDefault + // stronger DB assertions + assert Q.New(ZoneVO.class).count() == 1 + assert Q.New(ZoneVO.class).eq(ZoneVO_.isDefault, true).count() == 1 }compute/src/main/java/org/zstack/compute/zone/ZoneManagerImpl.java (1)
8-8: 避免通配符导入，保持显式依赖，便于可读性与静态检查请改回显式导入（同时补充对 Q 的导入），以符合常见 Checkstyle 规则并减少不必要的编译影响。
-import org.zstack.core.db.*; +import org.zstack.core.db.DatabaseFacade; +import org.zstack.core.db.DbEntityLister; +import org.zstack.core.db.Q; +import org.zstack.core.db.SQL; +import org.zstack.core.db.SQLBatch;
📜 Review details
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
compute/src/main/java/org/zstack/compute/zone/ZoneManagerImpl.java(2 hunks)test/src/test/groovy/org/zstack/test/integration/compute/ZoneCase.groovy(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
test/src/test/groovy/org/zstack/test/integration/compute/ZoneCase.groovycompute/src/main/java/org/zstack/compute/zone/ZoneManagerImpl.java
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名：
- API 名称必须唯一，不能重复。
- API 消息类需要继承
APIMessage；其返回类必须继承APIReply或APIEvent，并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest，并满足如下规范：
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时，使用
{variableName}格式。- HTTP 方法对应：
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档，并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名：
- 使用 UpperCamelCase 风格。
- 特殊情况：
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量：
- 使用 lowerCamelCase 风格。
常量命名：
- 全部大写，使用下划线分隔单词。
- 要求表达清楚，避免使用含糊或不准确的名称。
包名：
- 统一使用小写，使用点分隔符，每个部分应是一个具有自然语义的英文单词（参考 Spring 框架的结构）。
命名细节：
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量，防止混淆。
- 命名缩写：
- 不允许使用不必要的缩写，如：
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达：
- 避免使用布尔型参数造成含义不明确。例如：
- 对于
stopAgent(boolean ignoreError)，建议拆分为不同函数（如stopAgentIgnoreError()），或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）。
- 避免使用魔法值（Magic Value）：
直接使用未经定义的数值或字符串（如 if (status == 5)）应替换为枚举或常量。
示例：
// 错误示例：魔法值
if (user.getStatus() == 5) { ... }
// 正确示例：常量或枚举
public static final int STATUS_ACTIVE = 5;
if (user.getStatus() == STATUS_ACTIVE) { ... }
// 或使用枚举
enum UserStatus { ACTIVE, INACTIVE }
注释：
- 代码应尽量做到自解释，对少于两行的说明可以直接写在代码中。
- 对于较长的注释，需要仔细校对并随代码更新，确保内容正确。
- 接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用：
- 应尽量减少 if...else 结构的使用，建议：
- 限制嵌套层级最多为两层，且内层不应再出现
else分支。- 尽早返回（Early Return），将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断：
- if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述。
代码块长度：
...
Files:
compute/src/main/java/org/zstack/compute/zone/ZoneManagerImpl.java
Comment on lines
160
to
164
| Long existCount = Q.New(ZoneInventory.class).count(); | ||
| if (existCount == 0) { | ||
| msg.setDefault(true); | ||
| } | ||
|
|
There was a problem hiding this comment.
错误的实体类型：Q.New(ZoneInventory.class) 非持久化实体，编译/运行将失败
Q 查询需作用于 VO/EO。应改为 ZoneVO.class。同时，这段预检查放在串行队列之外，会引入竞态条件。
最小修复（仅实体类型）：
- Long existCount = Q.New(ZoneInventory.class).count();
+ long existingCount = Q.New(ZoneVO.class).count();
- if (existCount == 0) {
+ if (existingCount == 0) {
msg.setDefault(true);
}推荐修复（同时消除并发竞态：仅当“当前为零条”时，在串行队列内将首个 Zone 置为默认；否则保持调用方语义不变）：
- private void createZone(APICreateZoneMsg msg, ReturnValueCompletion<ZoneInventory> completion) {
- Long existCount = Q.New(ZoneInventory.class).count();
- if (existCount == 0) {
- msg.setDefault(true);
- }
-
- if (msg.getDefault() == null || !msg.getDefault()) {
- completion.success(createZoneFromApiMessage(msg));
- return;
- }
-
- thdf.chainSubmit(new ChainTask(msg) {
+ private void createZone(APICreateZoneMsg msg, ReturnValueCompletion<ZoneInventory> completion) {
+ long existingCount = Q.New(ZoneVO.class).count();
+ boolean needQueue = Boolean.TRUE.equals(msg.getDefault()) || existingCount == 0;
+
+ if (!needQueue) {
+ completion.success(createZoneFromApiMessage(msg));
+ return;
+ }
+
+ thdf.chainSubmit(new ChainTask(msg) {
@Override
public String getSyncSignature() {
return "default-zone-operation-queue";
}
@Override
public void run(SyncTaskChain chain) {
- completion.success(createZoneFromApiMessage(msg));
+ // Re-check under the queue to avoid TOCTOU
+ long currentCount = Q.New(ZoneVO.class).count();
+ if (currentCount == 0) {
+ msg.setDefault(true);
+ }
+ completion.success(createZoneFromApiMessage(msg));
chain.next();
}
@Override
public String getName() {
return "create-zone";
}
});
}说明：
- outside-queue 仅作快速判定以决定是否入队；真正是否设为默认在队列内再次判定，保证只有第一个任务在空表时将其设为默认，不会被后续并发创建覆盖。
test/src/test/groovy/org/zstack/test/integration/compute/ZoneCase.groovy
Outdated
Show resolved
Hide resolved
MatheMatrix
force-pushed
the
sync/lin.ma/ZSTAC-75952
branch
from
d593fc6 to
e4390cd
Compare
Resolves: ZSTAC-75952 Change-Id: I6c7a6f627a6c76796e656c6b7962796a7570736b
MatheMatrix
force-pushed
the
sync/lin.ma/ZSTAC-75952
branch
from
e4390cd to
db7e3de
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: ZSTAC-75952
Change-Id: I6c7a6f627a6c76796e656c6b7962796a7570736b
sync from gitlab !8435