<refactor>[zstack]: Refactor extension points #3257
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
Change-Id: I7a7678786d6e657369657a746170716775746f63
Walkthrough该变更引入了新的虚拟机和卷扩展点架构，并弃用了旧的扩展接口。新增了多个上下文数据类和统一的扩展点定义，同时用@deprecated注解标记了大量旧接口，指向新的扩展包中的替代方案。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 4
🤖 Fix all issues with AI agents
In
`@header/src/main/java/org/zstack/header/storage/snapshot/group/MemorySnapshotValidatorExtensionPoint.java`:
- Around line 6-15: Translate the Javadoc for
MemorySnapshotValidatorExtensionPoint into clear, idiomatic English (replace the
current Chinese text) so it documents the extension point purpose, trigger
timing (before creating a memory snapshot), the invocation site
(VolumeSnapshotGroupCreator.validate()), and usage scenario (modules check VM
external devices or network references for compatibility with memory snapshots);
keep the `@since` tag and ensure no Chinese remains in the comment.
In
`@header/src/main/java/org/zstack/header/vm/ResourceConfigMemorySnapshotExtensionPoint.java`:
- Around line 5-16: Translate the Javadoc for the
ResourceConfigMemorySnapshotExtensionPoint interface from Chinese to clear,
idiomatic English while preserving the structure and tags (including the
description, trigger/call location lines, usage scenario paragraph, and the
`@since` 5.0.0 tag); update the comment text to explain that this extension point
archives VM-related resource configuration when creating memory snapshots, is
invoked by MemorySnapshotManager.archiveResourceConfig(), and that modules
should provide configuration to be restored with the snapshot. Ensure no Chinese
remains and keep formatting and annotations intact.
In
`@header/src/main/java/org/zstack/header/vm/VmJustAfterDeleteFromDbExtensionPoint.java`:
- Around line 3-6: The `@deprecated` Javadoc in
VmJustAfterDeleteFromDbExtensionPoint currently links to the context class
VmDbDeleteContext instead of the actual single-phase extension point interface;
update the `@deprecated` tag to point to the concrete replacement extension point
in org.zstack.header.vm.extensions (replace the VmDbDeleteContext reference with
the correct extension-point interface name), and apply the identical change to
VmJustBeforeDeleteFromDbExtensionPoint so both deprecation comments reference
the explicit replacement interface rather than the context class.
In
`@header/src/main/java/org/zstack/header/vm/VmJustBeforeDeleteFromDbExtensionPoint.java`:
- Around line 3-6: Update the deprecation Javadoc on
VmJustBeforeDeleteFromDbExtensionPoint to name the exact single-phase
extension-point interface developers should migrate to (in addition to the
VmDbDeleteContext). Replace the vague phrase "with a single-phase extension
point" with a concrete fully-qualified interface reference (for example:
org.zstack.header.vm.extensions.<ExactExtensionPointInterfaceName>) so the
comment clearly shows the target extension-point to use when migrating.
🧹 Nitpick comments (2)
header/src/main/java/org/zstack/header/vm/extensions/VmDestroyExtensionPoint.java (1)
5-24: LGTM! 新的统一扩展点设计良好。接口设计符合重构规范：
- 四阶段方法命名（pre/before/after/failedTo）遵循设计规范
- 使用
VmDestroyContext上下文对象传递参数，便于未来扩展afterDestroy使用Completion回调支持异步处理- 默认实现合理（preDestroy 返回 null 表示不拒绝，afterDestroy 默认调用 success）
可选改进：根据文档要求，考虑添加
@since标签标注版本信息。header/src/main/java/org/zstack/header/vm/extensions/VmDestroyContext.java (1)
9-25: 建议添加类级别 Javadoc 文档以保持一致性。
VmAttachL3NetworkContext包含了完整的类级别 Javadoc 说明 Context 对象的用途和生命周期阶段，但此类缺少相应文档。建议保持统一风格：📝 建议添加文档
+/** + * Context object for VM destroy operations. + * + * <p>This context is passed through all lifecycle phases (pre/before/after/failedTo) + * allowing extensions to share state between phases.</p> + */ public class VmDestroyContext {
Comment on lines
+6
to
+15
| * 内存快照验证扩展点 | ||
| * | ||
| * <p>触发时机：创建内存快照前，验证 VM 是否符合条件</p> | ||
| * <p>调用位置：VolumeSnapshotGroupCreator.validate()</p> | ||
| * | ||
| * <h3>使用场景：</h3> | ||
| * <p>各模块通过此扩展点检查 VM 的外部设备或网络引用 | ||
| * 是否与内存快照兼容。</p> | ||
| * | ||
| * @since 5.0.0 |
There was a problem hiding this comment.
Javadoc 应使用英文编写
根据编码规范，代码中不应包含中文，包括注释和错误消息都应当使用正确的、无拼写错误的英文来编写。请将此 Javadoc 翻译为英文。
建议的英文 Javadoc
-/**
- * 内存快照验证扩展点
- *
- * <p>触发时机：创建内存快照前，验证 VM 是否符合条件</p>
- * <p>调用位置：VolumeSnapshotGroupCreator.validate()</p>
- *
- * <h3>使用场景：</h3>
- * <p>各模块通过此扩展点检查 VM 的外部设备或网络引用
- * 是否与内存快照兼容。</p>
- *
- * `@since` 5.0.0
- */
+/**
+ * Extension point for validating memory snapshot creation.
+ *
+ * <p>Trigger: Before creating a memory snapshot, validate whether the VM meets the conditions.</p>
+ * <p>Call site: VolumeSnapshotGroupCreator.validate()</p>
+ *
+ * <h3>Usage:</h3>
+ * <p>Modules use this extension point to check whether the VM's external devices
+ * or network references are compatible with memory snapshots.</p>
+ *
+ * `@since` 5.0.0
+ */根据编码规范：代码里不应当有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写。
🤖 Prompt for AI Agents
In
`@header/src/main/java/org/zstack/header/storage/snapshot/group/MemorySnapshotValidatorExtensionPoint.java`
around lines 6 - 15, Translate the Javadoc for
MemorySnapshotValidatorExtensionPoint into clear, idiomatic English (replace the
current Chinese text) so it documents the extension point purpose, trigger
timing (before creating a memory snapshot), the invocation site
(VolumeSnapshotGroupCreator.validate()), and usage scenario (modules check VM
external devices or network references for compatibility with memory snapshots);
keep the `@since` tag and ensure no Chinese remains in the comment.
Comment on lines
+5
to
+16
| /** | ||
| * 内存快照资源配置归档扩展点 | ||
| * | ||
| * <p>触发时机：创建内存快照时，归档与 VM 相关的资源配置</p> | ||
| * <p>调用位置：MemorySnapshotManager.archiveResourceConfig()</p> | ||
| * | ||
| * <h3>使用场景：</h3> | ||
| * <p>当 VM 执行内存快照时，各模块通过此扩展点提供需要归档的资源配置， | ||
| * 以便恢复快照时可以同时恢复相关配置状态。</p> | ||
| * | ||
| * @since 5.0.0 | ||
| */ |
There was a problem hiding this comment.
Javadoc 应使用英文编写
根据编码规范，代码中不应包含中文，包括注释和错误消息都应当使用正确的、无拼写错误的英文来编写。请将此 Javadoc 翻译为英文。
建议的英文 Javadoc
-/**
- * 内存快照资源配置归档扩展点
- *
- * <p>触发时机：创建内存快照时，归档与 VM 相关的资源配置</p>
- * <p>调用位置：MemorySnapshotManager.archiveResourceConfig()</p>
- *
- * <h3>使用场景：</h3>
- * <p>当 VM 执行内存快照时，各模块通过此扩展点提供需要归档的资源配置，
- * 以便恢复快照时可以同时恢复相关配置状态。</p>
- *
- * `@since` 5.0.0
- */
+/**
+ * Extension point for archiving resource configurations during memory snapshot creation.
+ *
+ * <p>Trigger: When creating a memory snapshot, archive VM-related resource configurations.</p>
+ * <p>Call site: MemorySnapshotManager.archiveResourceConfig()</p>
+ *
+ * <h3>Usage:</h3>
+ * <p>When a VM performs a memory snapshot, modules provide resource configurations
+ * to be archived via this extension point, allowing related configuration states
+ * to be restored when the snapshot is recovered.</p>
+ *
+ * `@since` 5.0.0
+ */根据编码规范：代码里不应当有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写。
🤖 Prompt for AI Agents
In
`@header/src/main/java/org/zstack/header/vm/ResourceConfigMemorySnapshotExtensionPoint.java`
around lines 5 - 16, Translate the Javadoc for the
ResourceConfigMemorySnapshotExtensionPoint interface from Chinese to clear,
idiomatic English while preserving the structure and tags (including the
description, trigger/call location lines, usage scenario paragraph, and the
`@since` 5.0.0 tag); update the comment text to explain that this extension point
archives VM-related resource configuration when creating memory snapshots, is
invoked by MemorySnapshotManager.archiveResourceConfig(), and that modules
should provide configuration to be restored with the snapshot. Ensure no Chinese
remains and keep formatting and annotations intact.
Comment on lines
3
to
+6
| /** | ||
| * Created by Qi Le on 2019-08-21 | ||
| * @deprecated Use {@link org.zstack.header.vm.extensions.VmDbDeleteContext} with a single-phase extension point instead. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
弃用引用不完整：与 VmJustBeforeDeleteFromDbExtensionPoint 存在相同问题。
同样引用了上下文类 VmDbDeleteContext 而非具体的扩展点接口。建议统一修复，指向明确的替代扩展点接口。
🤖 Prompt for AI Agents
In
`@header/src/main/java/org/zstack/header/vm/VmJustAfterDeleteFromDbExtensionPoint.java`
around lines 3 - 6, The `@deprecated` Javadoc in
VmJustAfterDeleteFromDbExtensionPoint currently links to the context class
VmDbDeleteContext instead of the actual single-phase extension point interface;
update the `@deprecated` tag to point to the concrete replacement extension point
in org.zstack.header.vm.extensions (replace the VmDbDeleteContext reference with
the correct extension-point interface name), and apply the identical change to
VmJustBeforeDeleteFromDbExtensionPoint so both deprecation comments reference
the explicit replacement interface rather than the context class.
Comment on lines
3
to
+6
| /** | ||
| * Created by xing5 on 2017/6/26. | ||
| * @deprecated Use {@link org.zstack.header.vm.extensions.VmDbDeleteContext} with a single-phase extension point instead. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for VmDbDeleteContext and related extension point interfaces
echo "=== Searching for VmDbDeleteContext ==="
fd -e java . | xargs rg -l "VmDbDeleteContext" | head -20
echo -e "\n=== Searching for VmDbDelete*ExtensionPoint interfaces ==="
fd -e java . | xargs rg "interface.*VmDbDelete.*ExtensionPoint" -A 2
echo -e "\n=== Checking VmJustBeforeDeleteFromDbExtensionPoint ==="
fd -e java . -name "*VmJustBeforeDeleteFromDb*" -exec cat {} \;
echo -e "\n=== Comparing with similar deprecation patterns ==="
fd -e java . | xargs rg "@Deprecated" -B 2 | grep -A 2 "VmDeleteExtensionPoint\|VmDbDelete" | head -30Repository: MatheMatrix/zstack
Length of output: 1558
🏁 Script executed:
#!/bin/bash
# Get the full content of VmJustBeforeDeleteFromDbExtensionPoint
echo "=== Full content of VmJustBeforeDeleteFromDbExtensionPoint ==="
cat header/src/main/java/org/zstack/header/vm/VmJustBeforeDeleteFromDbExtensionPoint.java
echo -e "\n=== Full content of VmDbDeleteContext ==="
cat header/src/main/java/org/zstack/header/vm/extensions/VmDbDeleteContext.java
echo -e "\n=== Check for all extension points related to VmDelete ==="
fd -e java . | xargs rg "interface.*ExtensionPoint" | grep -i "vmdelete\|vmdestroy\|vm.*delete" | head -20
echo -e "\n=== Check VmJustAfterDeleteFromDbExtensionPoint ==="
cat header/src/main/java/org/zstack/header/vm/VmJustAfterDeleteFromDbExtensionPoint.javaRepository: MatheMatrix/zstack
Length of output: 2425
🏁 Script executed:
#!/bin/bash
# Check VmDestroyExtensionPoint to see what it looks like
echo "=== Full content of VmDestroyExtensionPoint ==="
cat header/src/main/java/org/zstack/header/vm/extensions/VmDestroyExtensionPoint.java
echo -e "\n=== Search for VmDbDeleteExtensionPoint ==="
fd -e java . | xargs rg "VmDbDeleteExtensionPoint" 2>/dev/null || echo "Not found"
echo -e "\n=== Search for extension points that use VmDbDeleteContext ==="
fd -e java . | xargs rg "VmDbDeleteContext" -A 5 -B 5Repository: MatheMatrix/zstack
Length of output: 4431
🏁 Script executed:
#!/bin/bash
# Search for where VmJustBeforeDeleteFromDbExtensionPoint is actually used/implemented
echo "=== Usages of VmJustBeforeDeleteFromDbExtensionPoint ==="
fd -e java . | xargs rg "VmJustBeforeDeleteFromDbExtensionPoint" | grep -v "\.java:-"
echo -e "\n=== Look for recent VM delete/destroy extension point implementations ==="
fd -e java . | xargs rg "implements.*Delete.*ExtensionPoint\|implements.*Destroy.*ExtensionPoint" | head -20
echo -e "\n=== Search for pattern: what is the actual replacement mentioned in code ==="
fd -e java . | xargs rg "single-phase extension point" -B 3 -A 3Repository: MatheMatrix/zstack
Length of output: 3216
🏁 Script executed:
#!/bin/bash
# Check if there's any mention of what to use instead or if there's documentation
echo "=== Check for VmDbDelete* in extensions package ==="
fd -e java . header/src/main/java/org/zstack/header/vm/extensions/ | xargs ls -la
echo -e "\n=== Check VmDestroyContext to see how it differs ==="
find . -name "VmDestroyContext.java" -exec cat {} \;
echo -e "\n=== Check if there are any recent changes or comments about the replacement ==="
fd -e java . | xargs rg "VmDestroyExtensionPoint\|VmDbDelete" -B 2 -A 2 | grep -E "(TODO|FIXME|NOTE|phase)" | head -20Repository: MatheMatrix/zstack
Length of output: 2470
🏁 Script executed:
#!/bin/bash
# Check if VmDbDeleteExtensionPoint is planned/mentioned anywhere
echo "=== Search for any TODO, FIXME, or future references to VmDbDelete ==="
fd -e java . | xargs rg "VmDbDelete" -B 3 -A 3
echo -e "\n=== Check git history or comments about when VmDbDeleteContext was added ==="
git log --oneline --all -- "header/src/main/java/org/zstack/header/vm/extensions/VmDbDeleteContext.java" | head -10
echo -e "\n=== Look for pattern in similar deprecated interfaces ==="
fd -e java . | xargs rg "@Deprecated" -B 3 | grep -E "Use.*Context.*extension" -B 3 -A 3Repository: MatheMatrix/zstack
Length of output: 3792
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or comments about VmDbDeleteContext usage
echo "=== Search for implementations or usages of VmDbDeleteContext ==="
fd -e java . | xargs rg "VmDbDeleteContext" -A 5 | grep -v "^header/" | head -40
echo -e "\n=== Check the refactor commit message ==="
git show 1a4fa661c --stat | head -50
echo -e "\n=== Check related extension point interfaces with multiple phases ==="
cat header/src/main/java/org/zstack/header/vm/extensions/VmExpungeExtensionPoint.javaRepository: MatheMatrix/zstack
Length of output: 3974
弃用说明缺少具体的扩展点接口名称。
当前弃用注释引用了 VmDbDeleteContext（上下文类），但未指明对应的扩展点接口。注释仅说"with a single-phase extension point"而未命名具体的扩展点，这会导致开发者无法明确迁移目标。应补充具体的扩展点接口名称，使迁移路径清晰。
🤖 Prompt for AI Agents
In
`@header/src/main/java/org/zstack/header/vm/VmJustBeforeDeleteFromDbExtensionPoint.java`
around lines 3 - 6, Update the deprecation Javadoc on
VmJustBeforeDeleteFromDbExtensionPoint to name the exact single-phase
extension-point interface developers should migrate to (in addition to the
VmDbDeleteContext). Replace the vague phrase "with a single-phase extension
point" with a concrete fully-qualified interface reference (for example:
org.zstack.header.vm.extensions.<ExactExtensionPointInterfaceName>) so the
comment clearly shows the target extension-point to use when migrating.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Change-Id: I7a7678786d6e657369657a746170716775746f63
sync from gitlab !9077