Using global to replace config in protocol

[nanjiek]

@#2741
Replace config with global while ensuring compatibility in protocol.go
protocol_test.go still have some problems about TestExportWithApplicationConfig and TestGetProviderUrlWithHideKey

[nanjiek]

1. Where the error occurs

The error happens in registry/base_configuration_listener.go:65, inside the method BaseConfigurationListener.InitWith():

if rawConfig, err := bcl.dynamicConfiguration.GetRule(key,
    config_center.WithGroup(constant.Dubbo)); err != nil {
    bcl.configurators = []config_center.Configurator{}
    return
} else if len(rawConfig) > 0 {
    if err := bcl.genConfiguratorFromRawRule(rawConfig); err != nil {
        logger.Error("bcl.genConfiguratorFromRawRule(rawConfig:%v) = error:%v", rawConfig, err)
    }
}

---

2. Root cause — configuration format mismatch

The core issue is a format mismatch.

In config_center/mock_dynamic_config.go (lines 59–82), the MockDynamicConfiguration is initialized with content in properties format:

dynamicConfiguration.content = `
    dubbo.consumer.request_timeout=5s
    dubbo.consumer.connect_timeout=5s
    dubbo.application.organization=ikurento.com
    ...
`

However, in config_center/parser/configuration_parser.go, the ParseToUrls() method (line 118) expects the content to be in YAML format for the ConfiguratorConfig structure:

func (parser *DefaultConfigurationParser) ParseToUrls(content string) ([]*common.URL, error) {
    config := ConfiguratorConfig{}
    if err := yaml.Unmarshal([]byte(content), &config); err != nil {
        return nil, err
    }
    // ...
}

---

3. Error trigger flow

Here’s how the error happens step by step:

1. Initialization phase:
   The test calls exporterNormal() → regProtocol.Export() → which initializes BaseConfigurationListener.

2. Configuration retrieval:
   BaseConfigurationListener.InitWith() calls GetRule() to fetch the existing configuration from the mock config center.

3. Incorrect format returned:
   MockDynamicConfiguration.GetRule() returns the properties-format content.

4. Parsing failure:
   Inside genConfiguratorFromRawRule(), it calls ParseToUrls(), which tries to parse the properties string as YAML — causing a parsing error.

---

4. Why the test still passes

Even though an error is logged, the test passes successfully for several reasons:

1. Error is gracefully handled
   The failure in genConfiguratorFromRawRule() is only logged using logger.Error, not returned or propagated, so the test execution continues.

2. Subsequent correct configurations
   The test later calls MockServiceConfigEvent() and MockApplicationConfigEvent(), which send properly formatted YAML configurations:

   func (c *MockDynamicConfiguration) MockServiceConfigEvent() {
       config := &parser.ConfiguratorConfig{
           ConfigVersion: "2.7.1",
           Scope:         parser.GeneralType,
           Key:           mockServiceName,
           Enabled:       true,
           Configs: []parser.ConfigItem{...},
       }
       value, _ := yaml.Marshal(config)  // Correct YAML format
       // ...
   }

3. Final configuration takes effect
   Through the configuration event mechanism, the correctly formatted YAML content overrides the faulty one from initialization.
   As a result, the test assertions still succeed even though the earlier parsing error occurred.

---

[codecov-commenter]

Codecov Report

❌ Patch coverage is 54.34783% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.65%. Comparing base (1a7b169) to head (838ecab).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
registry/protocol/protocol.go	57.47%	32 Missing and 5 partials ⚠️
config_center/mock_dynamic_config.go	0.00%	5 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3054   +/-   ##
========================================
  Coverage    39.64%   39.65%           
========================================
  Files          457      457           
  Lines        39075    39133   +58     
========================================
+ Hits         15493    15517   +24     
- Misses       22318    22346   +28     
- Partials      1264     1270    +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull Request Overview

This PR migrates the protocol registry package from using the legacy config package to the newer global package for configuration management, while maintaining backward compatibility with existing code. The changes enable configurations to be passed via URL attributes instead of relying on global singleton config state.

Key Changes:

• Replaced config package imports with global package for configuration structures
• Modified protocol initialization to accept and use URL-embedded configuration attributes
• Updated test fixtures to provide configurations via URL attributes rather than global state

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.

File	Description
registry/protocol/protocol_test.go	Removed global config initialization; updated test helpers to pass configurations via URL attributes
registry/protocol/protocol.go	Added fallback logic to use global config from URLs with backward compatibility to config package
go.mod	Moved github.com/spf13/viper from indirect to direct dependency
common/constant/key.go	Added RpcServiceInterfaceName constant

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Alanxtl]

Suggested change

	providerConfig := config.GetProviderConfig()
	//TODO: Temporary compatibility with old APIs, can be removed later
	providerConfig := config.GetProviderConfig()

[Alanxtl]

need to make sure is ShutdownConfig already set into url or not

[nanjiek]

	if _, ok := registryUrl.GetAttribute(constant.ShutdownConfigPrefix); !ok {
		// Only set default global config when config package doesn't have one
		if config.GetShutDown() == nil {
			registryUrl.SetAttribute(constant.ShutdownConfigPrefix, global.DefaultShutdownConfig())
		}
	}   
```   In the front ， it has judgement.

[Alanxtl]

why closed

[nanjiek]

I will move the change to the fork of KamToHung