Skip to content

feat: 为状态机增加获取targetState、targetStateChain功能#518

Open
benym wants to merge 4 commits intoalibaba:masterfrom
benym:feat_getTargetState
Open

feat: 为状态机增加获取targetState、targetStateChain功能#518
benym wants to merge 4 commits intoalibaba:masterfrom
benym:feat_getTargetState

Conversation

@benym
Copy link
Copy Markdown

@benym benym commented Jun 12, 2024

虽然状态机在内部DSL维护了状态转移规则,但在实际使用过程中
发现不能够通过当前状态+事件获取到后续可能转移到的状态,DSL转移过程除UML图外不支持代码获取

比如场景

正向状态,待审核->业务审核通过->商务审核通过->法务审核通过

反向状态,待审核->业务审核驳回
待审核->业务审核通过->商务审核驳回
待审核->业务审核通过->商务审核通过->法务审核驳回

事件是审核通过、审核拒绝
在这个场景中通过待审核初始状态+审核通过事件,能够走到正向链路的最后,这条链路可能是需要展示出去的

更加通用的情况是:
通过当前状态+事件获取后续节点,或后续状态链
如果单独再维护一套等价于DSL转移过程的代码显得有点烦琐
除此之外也为了增加代码维度对后续状态获取的透明度,当状态机状态较多时提升状态链路可预测性

基于上述情况
新增4个功能

  1. com.alibaba.cola.statemachine.StateMachine#getTargetStates,支持从当前状态+事件获取DSL中下一跳状态
  2. com.alibaba.cola.statemachine.StateMachine#getTargetStateChain,支持从当前状态+事件获取DSL中的状态链,全面考虑现有internalTransition、externalTransition、externalTransitions、externalParallelTransition链路内部转化,一对一串行,一对多,多对一,环形链路等情况
  3. com.alibaba.cola.statemachine.StateChain#showStateChain,支持状态链的控制台打印可视化
  4. com.alibaba.cola.statemachine.StateChain#generatePlantUml,支持状态链PlantUml可视化

对应单元测试
com.alibaba.cola.test.StateChainTest

具体示例
给定如下状态机
image
获取初始状态为STATE1, 事件为EVENT1的,状态链为
image

feat: 为状态机增加预计算targetState功能,接口新增

feat: 完善stateChain测试用例及代码实现

feat: 移除多于代码

feat: 优化注释
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 12, 2024

CLA assistant check
All committers have signed the CLA.

@LeePui
Copy link
Copy Markdown

LeePui commented Oct 18, 2024

老哥,看你的状态描述和我的有点类似。我想咨询个问题,你上面说的:

比如场景
正向状态,待审核->业务审核通过->商务审核通过->法务审核通过
反向状态,待审核->业务审核驳回
待审核->业务审核通过->商务审核驳回
待审核->业务审核通过->商务审核通过->法务审核驳回
事件是审核通过、审核拒绝
在这个场景中通过待审核初始状态+审核通过事件,能够走到正向链路的最后,这条链路可能是需要展示出去的

  • 你这里只有两个事件:审核通过, 审核拒绝。按理说是不是还应该有一个开始审核事件,开始审核后,对象状态为审核中,然后经过审核,状态有可能变为:审核通过/审核拒绝
  • 假设是这样的话,那么结合COLA状态机,要怎么设计这个流程的代码呢?
  • 假设审核是一个接口方法:
// 对xxx文件进行审核的业务逻辑
booleab examine(xxx) {
    // 文件进入审核中状态
   // 开始审核
   if 某某条件: 
        审核通过:修改数据库文件状态,打印日志,发送文件审核通过消息
        触发stateMachine.fireEvent(审核通过)
    else:
        审核拒绝:修改数据库文件状态,打印日志,发送文件审核通过消息,发送手机短信告知
        触发stateMachine.fireEvent(审核拒绝)
}
  • 那么我想咨询下,在审核通过/审核拒绝的逻辑中,例如修改数据库文件状态,打印日志,发送文件审核通过消息,这些操作应该放到状态机对应的事件action里面去做,还是在examine方法里面做呢?
    • 假设在examine方法里面做,那触发stateMachine.fireEvent(审核通过)感觉就没啥用了
    • 假设在状态机对应的事件action里面去做,那么哪些应该在action里面做?哪些应该在examine方法里面做呢?
    • 想请教下你那边是怎么设计的

Copy link
Copy Markdown
Collaborator

@significantfrank significantfrank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR adds StateChain functionality to the state machine, enabling pre-calculation of subsequent states and state chains without performing actual transitions. The overall design with the Visitor pattern is solid.

Key Issues Found:

  1. Key collision risk in stateChainMap — simple string concatenation can cause collisions
  2. Only first internal transition handled in buildInternalStateChains — others are silently ignored
  3. Raw type usage State state without generics in BFS
  4. Thread safety — check-then-act pattern on ConcurrentHashMap is not atomic
  5. Typo "Event:+" should be "Event: "
  6. Misleading naminggetTargets() returns intermediate chain states, not just final targets
  7. No deduplication in getTargetStates() result

Suggestions:

  • Use computeIfAbsent for thread-safe lazy init of stateChainMap
  • Use a delimiter in cache keys to prevent collisions
  • Consider deduplicating target states or documenting the behavior
  • Align generic type conventions across StateChainVisitor methods

Copy link
Copy Markdown
Collaborator

@significantfrank significantfrank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skill verification test — please disregard.

Copy link
Copy Markdown
Collaborator

@significantfrank significantfrank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码检视总结

本 PR 为状态机增加了 getTargetStatesgetTargetStateChain 功能,整体设计思路清晰,新增的 Visitor 模式与已有代码保持了一致性。以下是主要发现:

🟠 Major (2)

  • BFS 算法逻辑问题buildExternalStateChains!stateQueue.contains(targetState) 条件会将非环路路径错误标记为环路,可能导致返回不正确的 StateChain
  • 原始类型使用getTargetStatesbuildExternalStateChains 中使用了原始类型 State 而非 State<S, E, C>,破坏了类型安全

🟡 Minor (7)

  • ConcurrentHashMap 的 check-then-act 竞态条件(建议用 computeIfAbsent
  • 缓存 key 缺少分隔符,存在碰撞风险
  • buildInternalStateChains 仅处理第一个内部转换
  • StateChainVisitor 泛型参数顺序 <S, C, E> 与项目惯例 <S, E, C> 不一致
  • Transition.getType() 声明后缺少空行
  • 通配符导入替换了原有具体导入
  • getTargetStatesState sourceState 使用原始类型

🔵 Suggestion (2)

  • 测试中 checkStateChain 的断言模式较弱,可能掩盖 bug
  • SysOutVisitor 的访问者方法同时产生副作用(打印)和返回值,建议分离

重点建议

最关键的问题是 BFS 算法的正确性,建议重点验证 buildExternalStateChains 在多路径场景下是否能产生正确结果。如果需要找到所有可能路径,建议改用 DFS + 回溯算法。

@benym
Copy link
Copy Markdown
Author

benym commented Apr 19, 2026

这个pr的时间实在是太久了,我重新回忆了下COLA的源码,对应的问题已经修复,环形链路的错误重新补了精确单测,部分是属于误判的

  • buildInternalStateChains只取第一个,这个原因是:内部转换的 source == target(同一状态),同一状态+同一事件只能有一个内部转换(构建状态机时会被com.alibaba.cola.statemachine.impl.EventTransitions#verify拒绝,这是既有逻辑)。因此只取第一个是正确且足够的
  • SysOutVisitor的打印问题,同时输出到控制台和字符串append是历史Visitor的行为,这里是为了保持一致
    有时间再CR一下,看还有什么地方需要改动 @significantfrank

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants