Skip to content

Conversation

WangzJi
Copy link
Contributor

@WangzJi WangzJi commented Oct 17, 2025

fixes: #7641

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@WangzJi WangzJi changed the title bugfix: ensure environment is fully loaded before ConfigurationFactor… bugfix: ensure environment is fully loaded before ConfigurationFactory initialization Oct 17, 2025
Copy link

codecov bot commented Oct 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.42%. Comparing base (d239a81) to head (34e4cdb).

Additional details and impacted files
@@             Coverage Diff              @@
##                2.x    #7705      +/-   ##
============================================
+ Coverage     61.38%   61.42%   +0.03%     
- Complexity      684      688       +4     
============================================
  Files          1323     1323              
  Lines         49943    49941       -2     
  Branches       5885     5884       -1     
============================================
+ Hits          30657    30675      +18     
+ Misses        16524    16508      -16     
+ Partials       2762     2758       -4     
Files with missing lines Coverage Δ
...ot/autoconfigure/loader/SeataPropertiesLoader.java 63.88% <100.00%> (+0.73%) ⬆️

... and 12 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ObjectHolder.INSTANCE.setObject(
OBJECT_KEY_SPRING_CONFIGURABLE_ENVIRONMENT, applicationContext.getEnvironment());
}
ObjectHolder.INSTANCE.setObject(OBJECT_KEY_SPRING_CONFIGURABLE_ENVIRONMENT, environment);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why this change was made?
What's the difference between setting the environment only when it's empty and setting it unconditionally (without checking for emptiness)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@funky-eyes Thanks for the review!

The key point: Line 64 triggers ConfigurationFactory static initialization, which needs a complete environment.

With the old null check:

  • EnvironmentPostProcessor sets environment (without application.properties in Spring Cloud)
  • ApplicationContextInitializer skips updating (due to null check)
  • ConfigurationFactory gets incomplete environment → can't read seata.config.type from application.properties

Unconditional update ensures:

  • Always use ApplicationContextInitializer's environment (which has all property sources loaded)
  • Safe because it's always more complete than EnvironmentPostProcessor's

This explains why bootstrap.yml workaround works - it's loaded early enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, in Spring Cloud there can be multiple Environment instances, which is why the environment may be incomplete. Otherwise, as I understand it, the same instance shouldn't have this problem, right?

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.

seata2.5配置文件加载顺序有问题,导致SpringBootConfigurationProvider#getConfigFromEnvironment无法正常加载到配置

2 participants