optimize: optimize config compatible module #6403

slievrly · 2024-03-09T14:45:54Z

I have registered the PR changes.

Ⅰ. Describe what this PR did

optimize config compatible module

Ⅱ. 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

codecov-commenter · 2024-03-09T14:56:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.41%. Comparing base (e72babb) to head (3a90f32).
Report is 8 commits behind head on 2.x.

❗ Current head 3a90f32 differs from pull request most recent head 65406be. Consider uploading reports for the commit 65406be to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##                2.x    #6403      +/-   ##
============================================
+ Coverage     50.19%   50.41%   +0.21%     
- Complexity     5232     5252      +20     
============================================
  Files           942      941       -1     
  Lines         33221    33279      +58     
  Branches       4022     4035      +13     
============================================
+ Hits          16676    16777     +101     
+ Misses        14938    14876      -62     
- Partials       1607     1626      +19

Files	Coverage Δ
...rg/apache/seata/core/constants/DubboConstants.java	`0.00% <ø> (ø)`

... and 48 files with indirect coverage changes

compatible/src/main/java/io/seata/config/ConfigurationChangeEvent.java

# Conflicts: # changes/en-us/2.x.md # changes/zh-cn/2.x.md # compatible/src/main/java/io/seata/core/context/RootContext.java

# Conflicts: # changes/en-us/2.x.md

funky-eyes · 2024-04-28T01:52:09Z

config/seata-config-core/src/main/java/io/seata/config/Processor.java

- * 
- *
- *
+ * Notes: used for io.seata SPI interface
 */


Suggested change

*/

*/

@Deprecated

funky-eyes · 2024-04-28T01:52:19Z

config/seata-config-core/src/main/java/io/seata/config/ConfigurationProvider.java

 */
-public interface ConfigurationProvider extends org.apache.seata.config.ConfigurationProvider {
+public interface ConfigurationProvider {


Suggested change

public interface ConfigurationProvider {

@Deprecated

public interface ConfigurationProvider {

funky-eyes · 2024-04-28T01:52:27Z

config/seata-config-core/src/main/java/io/seata/config/ConfigurationChangeType.java

- * 
- */
-public interface ExtConfigurationProvider extends org.apache.seata.config.ExtConfigurationProvider {
+public enum ConfigurationChangeType {


Suggested change

public enum ConfigurationChangeType {

@Deprecated

public enum ConfigurationChangeType {

funky-eyes · 2024-04-28T01:52:34Z

config/seata-config-core/src/main/java/io/seata/config/ConfigurationChangeListener.java

+
+import org.apache.seata.common.thread.NamedThreadFactory;
+
+public interface ConfigurationChangeListener {


Suggested change

public interface ConfigurationChangeListener {

@Deprecated

public interface ConfigurationChangeListener {

funky-eyes · 2024-04-28T01:52:47Z

config/seata-config-core/src/main/java/io/seata/config/ConfigurationChangeEvent.java

+ */
+package io.seata.config;
+
+public class ConfigurationChangeEvent {


Suggested change

public class ConfigurationChangeEvent {

@Deprecated

public class ConfigurationChangeEvent {

funky-eyes · 2024-04-28T01:53:07Z

config/seata-config-core/src/main/java/io/seata/config/AbstractConfiguration.java

+import org.apache.seata.common.util.DurationUtil;
+import org.apache.seata.common.util.StringUtils;
+
+public abstract class AbstractConfiguration implements Configuration {


Suggested change

public abstract class AbstractConfiguration implements Configuration {

@Deprecated

public abstract class AbstractConfiguration implements Configuration {

funky-eyes · 2024-04-28T02:05:23Z

config/seata-config-core/src/main/java/io/seata/config/FileConfiguration.java

+ * /apache/shardingsphere/transaction/base/seata/at/SeataATShardingSphereTransactionManager.java
+ * 2.EnhancedServiceLoader.load(ExtConfigurationProvider.class).provide(configuration)
+ */
+public class FileConfiguration extends AbstractConfiguration {


Suggested change

public class FileConfiguration extends AbstractConfiguration {

@Deprecated

public class FileConfiguration extends AbstractConfiguration {

Based on the conclusions from the previous meeting, Deprecated annotations will be updated uniformly.

config/seata-config-core/src/main/java/org/apache/seata/config/ConfigurationFactory.java

funky-eyes · 2024-04-28T02:19:20Z

config/seata-config-core/src/main/java/org/apache/seata/config/ConfigurationFactory.java

+        } catch (Exception exx) {
+            LOGGER.error("failed to load non-spring configuration :{}", exx.getMessage(), exx);
+        }
+        return ORIGIN_FILE_INSTANCE;


这里不应该返回ORIGIN_FILE_INSTANCE，更应该直接抛出异常提示，可以比较明确的判断问题
Here, instead of returning ORIGIN_FILE_INSTANCE, it would be more appropriate to directly throw an exception with a clear error message, allowing for a more straightforward identification of the issue.

An exception cannot be thrown here because the file mode does not go through SPI. I've broken down the previous logic a bit and extracted the assignment of default values to the outer logic, but overall, it remains consistent with what was there before.

config/seata-config-core/src/main/java/org/apache/seata/config/ConfigurationFactory.java

funky-eyes

LGTM

funky-eyes reviewed Mar 10, 2024

View reviewed changes

compatible/src/main/java/io/seata/config/ConfigurationChangeEvent.java Outdated Show resolved Hide resolved

slievrly force-pushed the 2.x_24_03_09_01 branch from 9ab3e2f to 963026f Compare March 30, 2024 09:02

slievrly added 7 commits April 21, 2024 23:06

step 1

cd5e54b

step 2

05b9379

optimize: optimize config compatible module

a250459

# Conflicts: # changes/en-us/2.x.md # changes/zh-cn/2.x.md # compatible/src/main/java/io/seata/core/context/RootContext.java

optimize: optimize config compatible module

e66a3d2

# Conflicts: # changes/en-us/2.x.md # changes/zh-cn/2.x.md # compatible/src/main/java/io/seata/core/context/RootContext.java

temp

1fca17f

add test for config compatible

9494fb7

step 3

522563a

slievrly force-pushed the 2.x_24_03_09_01 branch from b36c756 to 522563a Compare April 21, 2024 15:34

SPI definition use lowercase

eb2520c

slievrly added this to the 2.1.0 milestone Apr 22, 2024

Merge remote-tracking branch 'upstream/2.x' into 2.x_24_03_09_01

c9aa6d4

# Conflicts: # changes/en-us/2.x.md

funky-eyes added module/config config module type: optimize labels Apr 25, 2024

funky-eyes reviewed Apr 28, 2024

View reviewed changes

slievrly added 2 commits April 29, 2024 17:29

fix review

3b9fb1e

fix review

65406be

funky-eyes approved these changes Apr 30, 2024

View reviewed changes

slievrly merged commit 4f16097 into apache:2.x Apr 30, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

optimize: optimize config compatible module #6403

optimize: optimize config compatible module #6403

slievrly commented Mar 9, 2024

codecov-commenter commented Mar 9, 2024 •

edited

Loading

funky-eyes Apr 28, 2024

funky-eyes Apr 28, 2024

funky-eyes Apr 28, 2024

funky-eyes Apr 28, 2024

funky-eyes Apr 28, 2024

funky-eyes Apr 28, 2024

funky-eyes Apr 28, 2024

slievrly Apr 29, 2024

funky-eyes Apr 28, 2024

slievrly Apr 29, 2024

slievrly Apr 29, 2024

funky-eyes left a comment

	public interface ConfigurationProvider {
	@Deprecated
	public interface ConfigurationProvider {

	public enum ConfigurationChangeType {
	@Deprecated
	public enum ConfigurationChangeType {


		import org.apache.seata.common.thread.NamedThreadFactory;

		public interface ConfigurationChangeListener {

	public class ConfigurationChangeEvent {
	@Deprecated
	public class ConfigurationChangeEvent {

	public abstract class AbstractConfiguration implements Configuration {
	@Deprecated
	public abstract class AbstractConfiguration implements Configuration {

	public class FileConfiguration extends AbstractConfiguration {
	@Deprecated
	public class FileConfiguration extends AbstractConfiguration {

optimize: optimize config compatible module #6403

optimize: optimize config compatible module #6403

Conversation

slievrly commented Mar 9, 2024

Ⅰ. 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

codecov-commenter commented Mar 9, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

funky-eyes left a comment

Choose a reason for hiding this comment

codecov-commenter commented Mar 9, 2024 •

edited

Loading