Copilot commented on code in PR #3132: URL: https://github.com/apache/dubbo-go/pull/3132#discussion_r2637844911
########## server/compat_test.go: ########## @@ -0,0 +1,328 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package server + +import ( + "testing" +) + +import ( + "github.com/stretchr/testify/assert" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test compatApplicationConfig with all fields +func TestCompatApplicationConfigAll(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Organization: "test-org", + Name: "test-app", + Module: "test-module", + Group: "test-group", + Version: "1.0.0", + Owner: "test-owner", + Environment: "test-env", + MetadataType: "remote", + Tag: "v1", + MetadataServicePort: "30880", + MetadataServiceProtocol: "triple", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-org", result.Organization) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "test-module", result.Module) + assert.Equal(t, "test-group", result.Group) + assert.Equal(t, "1.0.0", result.Version) + assert.Equal(t, "test-owner", result.Owner) + assert.Equal(t, "test-env", result.Environment) + assert.Equal(t, "remote", result.MetadataType) + assert.Equal(t, "v1", result.Tag) + assert.Equal(t, "30880", result.MetadataServicePort) + assert.Equal(t, "triple", result.MetadataServiceProtocol) +} + +// Test compatApplicationConfig with partial fields +func TestCompatApplicationConfigPartial(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + Version: "1.0.0", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "1.0.0", result.Version) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) +} + +// Test compatApplicationConfig with empty config +func TestCompatApplicationConfigEmpty(t *testing.T) { + appCfg := &global.ApplicationConfig{} + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Name) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) + assert.Empty(t, result.Group) + assert.Empty(t, result.Version) + assert.Empty(t, result.Owner) + assert.Empty(t, result.Environment) + assert.Empty(t, result.MetadataType) + assert.Empty(t, result.Tag) + assert.Empty(t, result.MetadataServicePort) + assert.Empty(t, result.MetadataServiceProtocol) +} + +// Test compatRegistryConfig with all fields +func TestCompatRegistryConfigAll(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Timeout: "10s", + Group: "registry-group", + Namespace: "test-ns", + TTL: "60s", + Address: "localhost:2181", + Username: "admin", + Password: "password", + Simplified: true, + Preferred: true, + Zone: "zone1", + Weight: 100, + Params: map[string]string{"key": "value"}, + RegistryType: "service_discovery", + UseAsMetaReport: "true", + UseAsConfigCenter: "true", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + assert.Equal(t, "10s", result.Timeout) + assert.Equal(t, "registry-group", result.Group) + assert.Equal(t, "test-ns", result.Namespace) + assert.Equal(t, "60s", result.TTL) + assert.Equal(t, "localhost:2181", result.Address) + assert.Equal(t, "admin", result.Username) + assert.Equal(t, "password", result.Password) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "zone1", result.Zone) + // Verify weight is precisely preserved from original input + assert.Equal(t, int64(100), result.Weight) + assert.Equal(t, "service_discovery", result.RegistryType) + assert.Equal(t, "true", result.UseAsMetaReport) + assert.Equal(t, "true", result.UseAsConfigCenter) + assert.NotNil(t, result.Params) + assert.Equal(t, "value", result.Params["key"]) +} + +// Test compatRegistryConfig with partial fields +func TestCompatRegistryConfigPartial(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "nacos", + Address: "localhost:8848", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "nacos", result.Protocol) + assert.Equal(t, "localhost:8848", result.Address) + assert.Empty(t, result.Timeout) + assert.Empty(t, result.Group) +} + +// Test compatRegistryConfig with empty config +func TestCompatRegistryConfigEmpty(t *testing.T) { + regCfg := &global.RegistryConfig{} + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Protocol) + assert.Empty(t, result.Address) + assert.Empty(t, result.Timeout) + assert.Empty(t, result.Group) + assert.Empty(t, result.Namespace) + assert.Empty(t, result.TTL) + assert.Empty(t, result.Username) + assert.Empty(t, result.Password) + assert.False(t, result.Simplified) + assert.False(t, result.Preferred) +} + +// Test compatRegistryConfig with boolean fields +func TestCompatRegistryConfigBooleans(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "etcd", + Simplified: true, + Preferred: true, + UseAsMetaReport: "true", + UseAsConfigCenter: "false", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "true", result.UseAsMetaReport) + assert.Equal(t, "false", result.UseAsConfigCenter) +} + +// Test compatRegistryConfig with numeric fields +func TestCompatRegistryConfigNumeric(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Weight: 500, + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + // Verify weight is precisely preserved from original input + assert.Equal(t, int64(500), result.Weight) +} + +// Test compatRegistryConfig with params +func TestCompatRegistryConfigWithParams(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Params: map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }, + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, 3, len(result.Params)) + assert.Equal(t, "value1", result.Params["key1"]) + assert.Equal(t, "value2", result.Params["key2"]) + assert.Equal(t, "value3", result.Params["key3"]) +} + +// Test compatApplicationConfig preserves all field types +func TestCompatApplicationConfigFieldPreservation(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "myapp", + Organization: "myorg", + Module: "mymodule", + Group: "mygroup", + Version: "2.0.0", + Owner: "owner", + Environment: "production", + MetadataType: "local", + Tag: "beta", + MetadataServicePort: "25555", + MetadataServiceProtocol: "grpc", + } + + result := compatApplicationConfig(appCfg) + + // Verify type conversion doesn't lose data + assert.IsType(t, (*config.ApplicationConfig)(nil), result) + assert.Equal(t, appCfg.Name, result.Name) + assert.Equal(t, appCfg.Organization, result.Organization) + assert.Equal(t, appCfg.Module, result.Module) +} + +// Test compatRegistryConfig preserves all field types +func TestCompatRegistryConfigFieldPreservation(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "consul", + Timeout: "30s", + Group: "consul-group", + Namespace: "consul-ns", + TTL: "120s", + Address: "localhost:8500", + Username: "consul-user", + Password: "consul-pass", + Simplified: true, + Preferred: false, + Zone: "zone-a", + Weight: 200, + Params: map[string]string{"consul-key": "consul-val"}, + RegistryType: "interface", + UseAsMetaReport: "true", + UseAsConfigCenter: "false", + } + + result := compatRegistryConfig(regCfg) + + // Verify type conversion doesn't lose data Review Comment: The comment states "Verify type conversion doesn't lose data" but this is redundant and doesn't add meaningful documentation value. The test code itself is self-explanatory. This comment should be removed to improve code clarity. ########## server/compat_test.go: ########## @@ -0,0 +1,328 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package server + +import ( + "testing" +) + +import ( + "github.com/stretchr/testify/assert" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test compatApplicationConfig with all fields +func TestCompatApplicationConfigAll(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Organization: "test-org", + Name: "test-app", + Module: "test-module", + Group: "test-group", + Version: "1.0.0", + Owner: "test-owner", + Environment: "test-env", + MetadataType: "remote", + Tag: "v1", + MetadataServicePort: "30880", + MetadataServiceProtocol: "triple", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-org", result.Organization) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "test-module", result.Module) + assert.Equal(t, "test-group", result.Group) + assert.Equal(t, "1.0.0", result.Version) + assert.Equal(t, "test-owner", result.Owner) + assert.Equal(t, "test-env", result.Environment) + assert.Equal(t, "remote", result.MetadataType) + assert.Equal(t, "v1", result.Tag) + assert.Equal(t, "30880", result.MetadataServicePort) + assert.Equal(t, "triple", result.MetadataServiceProtocol) +} + +// Test compatApplicationConfig with partial fields +func TestCompatApplicationConfigPartial(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + Version: "1.0.0", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "1.0.0", result.Version) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) +} + +// Test compatApplicationConfig with empty config +func TestCompatApplicationConfigEmpty(t *testing.T) { + appCfg := &global.ApplicationConfig{} + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Name) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) + assert.Empty(t, result.Group) + assert.Empty(t, result.Version) + assert.Empty(t, result.Owner) + assert.Empty(t, result.Environment) + assert.Empty(t, result.MetadataType) + assert.Empty(t, result.Tag) + assert.Empty(t, result.MetadataServicePort) + assert.Empty(t, result.MetadataServiceProtocol) +} + +// Test compatRegistryConfig with all fields +func TestCompatRegistryConfigAll(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Timeout: "10s", + Group: "registry-group", + Namespace: "test-ns", + TTL: "60s", + Address: "localhost:2181", + Username: "admin", + Password: "password", + Simplified: true, + Preferred: true, + Zone: "zone1", + Weight: 100, + Params: map[string]string{"key": "value"}, + RegistryType: "service_discovery", + UseAsMetaReport: "true", + UseAsConfigCenter: "true", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + assert.Equal(t, "10s", result.Timeout) + assert.Equal(t, "registry-group", result.Group) + assert.Equal(t, "test-ns", result.Namespace) + assert.Equal(t, "60s", result.TTL) + assert.Equal(t, "localhost:2181", result.Address) + assert.Equal(t, "admin", result.Username) + assert.Equal(t, "password", result.Password) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "zone1", result.Zone) + // Verify weight is precisely preserved from original input Review Comment: The comment states "Verify weight is precisely preserved from original input" but this is redundant with the test itself and doesn't add meaningful documentation value. This comment should be removed to improve code clarity. ```suggestion ``` ########## server/action_test.go: ########## @@ -0,0 +1,705 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package server + +import ( + "testing" +) + +import ( + "github.com/stretchr/testify/assert" + + "go.uber.org/atomic" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/common" + "dubbo.apache.org/dubbo-go/v3/common/constant" + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/global" + "dubbo.apache.org/dubbo-go/v3/protocol/base" +) + +// Test Prefix method +func TestPrefix(t *testing.T) { + svcOpts := &ServiceOptions{ + Id: "com.example.TestService", + } + + prefix := svcOpts.Prefix() + assert.Equal(t, "dubbo.service.com.example.TestService", prefix) +} + +// Test InitExported +func TestInitExported(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(true), + } + + svcOpts.InitExported() + assert.False(t, svcOpts.exported.Load()) +} + +// Test IsExport returns false initially +func TestIsExportFalse(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(false), + } + + assert.False(t, svcOpts.IsExport()) +} + +// Test IsExport returns true when exported +func TestIsExportTrue(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(true), + } + + assert.True(t, svcOpts.IsExport()) +} + +// Test check with valid TpsLimiter +func TestCheckValidTpsLimiter(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimiter: "", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with valid TpsLimitRate +func TestCheckValidTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "1000", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with invalid TpsLimitRate +func TestCheckInvalidTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "invalid", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with negative TpsLimitRate +func TestCheckNegativeTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "-1", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with valid TpsLimitInterval +func TestCheckValidTpsLimitInterval(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitInterval: "1000", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with invalid TpsLimitInterval +func TestCheckInvalidTpsLimitInterval(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitInterval: "invalid", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with negative TpsLimitInterval +func TestCheckNegativeTpsLimitInterval(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitInterval: "-1", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test Implement +func TestImplement(t *testing.T) { + svcOpts := &ServiceOptions{} + mockService := &mockRPCService{} + + svcOpts.Implement(mockService) + + assert.Equal(t, mockService, svcOpts.rpcService) +} + +// Test Unexport when not exported +func TestUnexportNotExported(t *testing.T) { + svcOpts := &ServiceOptions{ + unexported: atomic.NewBool(false), + exported: atomic.NewBool(false), + exporters: []base.Exporter{}, + } + + svcOpts.Unexport() + assert.False(t, svcOpts.exported.Load()) + assert.False(t, svcOpts.unexported.Load()) +} + +// Test Unexport when already unexported +func TestUnexportAlreadyUnexported(t *testing.T) { + svcOpts := &ServiceOptions{ + unexported: atomic.NewBool(true), + exported: atomic.NewBool(false), + exporters: []base.Exporter{}, + } + + svcOpts.Unexport() + assert.True(t, svcOpts.unexported.Load()) +} + +// Test removeDuplicateElement +func TestRemoveDuplicateElement(t *testing.T) { + items := []string{"a", "b", "a", "c", "b", ""} + result := removeDuplicateElement(items) + + assert.Len(t, result, 3) + assert.Contains(t, result, "a") + assert.Contains(t, result, "b") + assert.Contains(t, result, "c") +} + +// Test removeDuplicateElement with empty slice +func TestRemoveDuplicateElementEmpty(t *testing.T) { + items := []string{} + result := removeDuplicateElement(items) + + assert.Len(t, result, 0) +} + +// Test removeDuplicateElement with only empty strings +func TestRemoveDuplicateElementOnlyEmpty(t *testing.T) { + items := []string{"", "", ""} + result := removeDuplicateElement(items) + + assert.Len(t, result, 0) +} + +// Test removeDuplicateElement with mixed content +func TestRemoveDuplicateElementMixed(t *testing.T) { + items := []string{"reg1", "reg2", "reg1", "", "reg3", ""} + result := removeDuplicateElement(items) + + assert.Len(t, result, 3) + assert.Contains(t, result, "reg1") + assert.Contains(t, result, "reg2") + assert.Contains(t, result, "reg3") +} + +// Test getRegistryIds +func TestGetRegistryIds(t *testing.T) { + registries := map[string]*global.RegistryConfig{ + "reg1": {Protocol: "zookeeper"}, + "reg2": {Protocol: "nacos"}, + "reg3": {Protocol: "etcd"}, + } + + ids := getRegistryIds(registries) + + assert.Len(t, ids, 3) + assert.Contains(t, ids, "reg1") + assert.Contains(t, ids, "reg2") + assert.Contains(t, ids, "reg3") +} + +// Test getRegistryIds with empty map +func TestGetRegistryIdsEmpty(t *testing.T) { + registries := map[string]*global.RegistryConfig{} + + ids := getRegistryIds(registries) + + assert.Len(t, ids, 0) +} + +// Test GetExportedUrls when not exported +func TestGetExportedUrlsNotExported(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(false), + exporters: []base.Exporter{}, + } + + urls := svcOpts.GetExportedUrls() + assert.Nil(t, urls) +} + +// Test GetExportedUrls when exported with no exporters +func TestGetExportedUrlsExportedEmpty(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(true), + exporters: []base.Exporter{}, + } + + urls := svcOpts.GetExportedUrls() + assert.Len(t, urls, 0) +} + +// Test getUrlMap basic functionality +func TestGetUrlMapBasic(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Filter: "", + Params: map[string]string{}, + NotRegister: false, + Tag: "v1", + Group: "test-group", + Version: "1.0.0", + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + Organization: "test-org", + Module: "test-module", + Owner: "test-owner", + Environment: "test-env", + Version: "1.0.0", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{Enable: nil}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{Enable: nil}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.NotNil(t, urlMap) + assert.Equal(t, "com.example.Service", urlMap.Get(constant.InterfaceKey)) + assert.Equal(t, constant.ClusterKeyFailover, urlMap.Get(constant.ClusterKey)) + assert.Equal(t, constant.LoadBalanceKeyRoundRobin, urlMap.Get(constant.LoadbalanceKey)) + assert.Equal(t, "60", urlMap.Get(constant.WarmupKey)) + assert.Equal(t, "3", urlMap.Get(constant.RetriesKey)) + assert.Equal(t, constant.JSONSerialization, urlMap.Get(constant.SerializationKey)) +} + +// Test getUrlMap with custom params +func TestGetUrlMapWithParams(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Params: map[string]string{ + "customKey": "customValue", + }, + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + Organization: "test-org", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.NotNil(t, urlMap) + assert.Equal(t, "customValue", urlMap.Get("customKey")) +} + +// Test getUrlMap with group and version +func TestGetUrlMapWithGroupAndVersion(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Group: "test-group", + Version: "1.0.0", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Params: map[string]string{}, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.Equal(t, "test-group", urlMap.Get(constant.GroupKey)) + assert.Equal(t, "1.0.0", urlMap.Get(constant.VersionKey)) +} + +// Test getUrlMap with methods +func TestGetUrlMapWithMethods(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Params: map[string]string{}, + Methods: []*global.MethodConfig{ + { + Name: "testMethod", + LoadBalance: "random", + Retries: "5", + Weight: 200, + }, + }, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.Equal(t, "random", urlMap.Get("methods.testMethod."+constant.LoadbalanceKey)) + assert.Equal(t, "5", urlMap.Get("methods.testMethod."+constant.RetriesKey)) + assert.Equal(t, "200", urlMap.Get("methods.testMethod."+constant.WeightKey)) +} + +// Test loadProtocol with matching IDs +func TestLoadProtocol(t *testing.T) { + protocols := map[string]*global.ProtocolConfig{ + "dubbo": { + Name: "dubbo", + Port: "20880", + }, + "triple": { + Name: "triple", + Port: "50051", + }, + } + + protocolIDs := []string{"dubbo", "triple"} + result := loadProtocol(protocolIDs, protocols) + + assert.Len(t, result, 2) +} + +// Test loadProtocol with partial matching +func TestLoadProtocolPartialMatch(t *testing.T) { + protocols := map[string]*global.ProtocolConfig{ + "dubbo": { + Name: "dubbo", + Port: "20880", + }, + "triple": { + Name: "triple", + Port: "50051", + }, + } + + protocolIDs := []string{"dubbo"} + result := loadProtocol(protocolIDs, protocols) + + assert.Len(t, result, 1) + assert.Equal(t, "dubbo", result[0].Name) +} + +// Test loadProtocol with no matching IDs +func TestLoadProtocolNoMatch(t *testing.T) { + protocols := map[string]*global.ProtocolConfig{ + "dubbo": { + Name: "dubbo", + Port: "20880", + }, + } + + protocolIDs := []string{"non-existent"} + result := loadProtocol(protocolIDs, protocols) + + assert.Len(t, result, 0) +} + +// Test setRegistrySubURL +func TestSetRegistrySubURL(t *testing.T) { + ivkURL, _ := common.NewURL("dubbo://localhost:20880/com.example.Service") + ivkURL.AddParam(constant.RegistryKey, "zookeeper") + ivkURL.AddParam(constant.RegistryTypeKey, "service_discovery") + + regURL, _ := common.NewURL("registry://zookeeper:2181") + regURL.AddParam(constant.RegistryKey, "zookeeper") + regURL.AddParam(constant.RegistryTypeKey, "service_discovery") + + setRegistrySubURL(ivkURL, regURL) + + assert.Equal(t, "zookeeper", ivkURL.GetParam(constant.RegistryKey, "")) + assert.Equal(t, "service_discovery", ivkURL.GetParam(constant.RegistryTypeKey, "")) + assert.NotNil(t, regURL.SubURL) +} + +// Test Unexport when exported +func TestUnexportWhenExported(t *testing.T) { + svcOpts := &ServiceOptions{ + unexported: atomic.NewBool(false), + exported: atomic.NewBool(true), + exporters: []base.Exporter{}, + } + + svcOpts.Unexport() + assert.False(t, svcOpts.exported.Load()) + assert.True(t, svcOpts.unexported.Load()) +} + +// Test check with multiple validation errors +func TestCheckMultipleErrors(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "invalid", + TpsLimitInterval: "also-invalid", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test getUrlMap with tag +func TestGetUrlMapWithTag(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Tag: "production", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Params: map[string]string{}, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + // Tag is added separately in Export method, not in getUrlMap + assert.Equal(t, "production", svcOpts.Service.Tag) +} + +// Test getUrlMap with TPS limit config +func TestGetUrlMapWithTpsLimit(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimiter: "default", + TpsLimitRate: "1000", + TpsLimitStrategy: "adaptive", + TpsLimitRejectedHandler: "abort", + TpsLimitInterval: "100", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Params: map[string]string{}, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.Equal(t, "default", urlMap.Get(constant.TPSLimiterKey)) + assert.Equal(t, "1000", urlMap.Get(constant.TPSLimitRateKey)) + assert.Equal(t, "adaptive", urlMap.Get(constant.TPSLimitStrategyKey)) + assert.Equal(t, "abort", urlMap.Get(constant.TPSRejectedExecutionHandlerKey)) +} + +// Test getUrlMap with execute limit config +func TestGetUrlMapWithExecuteLimit(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + ExecuteLimit: "200", + ExecuteLimitRejectedHandler: "abort", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Params: map[string]string{}, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.Equal(t, "200", urlMap.Get(constant.ExecuteLimitKey)) + assert.Equal(t, "abort", urlMap.Get(constant.ExecuteRejectedExecutionHandlerKey)) +} + +// Test getUrlMap with auth and param sign +func TestGetUrlMapWithAuthAndParamSign(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Auth: "default", + ParamSign: "md5", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Params: map[string]string{}, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.Equal(t, "default", urlMap.Get(constant.ServiceAuthKey)) + assert.Equal(t, "md5", urlMap.Get(constant.ParameterSignatureEnableKey)) +} + +// Test getUrlMap with access log +func TestGetUrlMapWithAccessLog(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + AccessLog: "/var/log/access.log", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Params: map[string]string{}, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.Equal(t, "/var/log/access.log", urlMap.Get(constant.AccessLogFilterKey)) +} + +// Test postProcessConfig +func TestPostProcessConfig(t *testing.T) { + svcOpts := &ServiceOptions{} + url, _ := common.NewURL("dubbo://localhost:20880/test") + + // This should not panic Review Comment: The comment "This should not panic" doesn't add meaningful documentation value. The test already makes this obvious by not expecting a panic. This comment should be removed to improve code clarity. ```suggestion ``` ########## server/compat_test.go: ########## @@ -0,0 +1,328 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package server + +import ( + "testing" +) + +import ( + "github.com/stretchr/testify/assert" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test compatApplicationConfig with all fields +func TestCompatApplicationConfigAll(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Organization: "test-org", + Name: "test-app", + Module: "test-module", + Group: "test-group", + Version: "1.0.0", + Owner: "test-owner", + Environment: "test-env", + MetadataType: "remote", + Tag: "v1", + MetadataServicePort: "30880", + MetadataServiceProtocol: "triple", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-org", result.Organization) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "test-module", result.Module) + assert.Equal(t, "test-group", result.Group) + assert.Equal(t, "1.0.0", result.Version) + assert.Equal(t, "test-owner", result.Owner) + assert.Equal(t, "test-env", result.Environment) + assert.Equal(t, "remote", result.MetadataType) + assert.Equal(t, "v1", result.Tag) + assert.Equal(t, "30880", result.MetadataServicePort) + assert.Equal(t, "triple", result.MetadataServiceProtocol) +} + +// Test compatApplicationConfig with partial fields +func TestCompatApplicationConfigPartial(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + Version: "1.0.0", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "1.0.0", result.Version) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) +} + +// Test compatApplicationConfig with empty config +func TestCompatApplicationConfigEmpty(t *testing.T) { + appCfg := &global.ApplicationConfig{} + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Name) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) + assert.Empty(t, result.Group) + assert.Empty(t, result.Version) + assert.Empty(t, result.Owner) + assert.Empty(t, result.Environment) + assert.Empty(t, result.MetadataType) + assert.Empty(t, result.Tag) + assert.Empty(t, result.MetadataServicePort) + assert.Empty(t, result.MetadataServiceProtocol) +} + +// Test compatRegistryConfig with all fields +func TestCompatRegistryConfigAll(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Timeout: "10s", + Group: "registry-group", + Namespace: "test-ns", + TTL: "60s", + Address: "localhost:2181", + Username: "admin", + Password: "password", + Simplified: true, + Preferred: true, + Zone: "zone1", + Weight: 100, + Params: map[string]string{"key": "value"}, + RegistryType: "service_discovery", + UseAsMetaReport: "true", + UseAsConfigCenter: "true", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + assert.Equal(t, "10s", result.Timeout) + assert.Equal(t, "registry-group", result.Group) + assert.Equal(t, "test-ns", result.Namespace) + assert.Equal(t, "60s", result.TTL) + assert.Equal(t, "localhost:2181", result.Address) + assert.Equal(t, "admin", result.Username) + assert.Equal(t, "password", result.Password) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "zone1", result.Zone) + // Verify weight is precisely preserved from original input + assert.Equal(t, int64(100), result.Weight) + assert.Equal(t, "service_discovery", result.RegistryType) + assert.Equal(t, "true", result.UseAsMetaReport) + assert.Equal(t, "true", result.UseAsConfigCenter) + assert.NotNil(t, result.Params) + assert.Equal(t, "value", result.Params["key"]) +} + +// Test compatRegistryConfig with partial fields +func TestCompatRegistryConfigPartial(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "nacos", + Address: "localhost:8848", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "nacos", result.Protocol) + assert.Equal(t, "localhost:8848", result.Address) + assert.Empty(t, result.Timeout) + assert.Empty(t, result.Group) +} + +// Test compatRegistryConfig with empty config +func TestCompatRegistryConfigEmpty(t *testing.T) { + regCfg := &global.RegistryConfig{} + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Protocol) + assert.Empty(t, result.Address) + assert.Empty(t, result.Timeout) + assert.Empty(t, result.Group) + assert.Empty(t, result.Namespace) + assert.Empty(t, result.TTL) + assert.Empty(t, result.Username) + assert.Empty(t, result.Password) + assert.False(t, result.Simplified) + assert.False(t, result.Preferred) +} + +// Test compatRegistryConfig with boolean fields +func TestCompatRegistryConfigBooleans(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "etcd", + Simplified: true, + Preferred: true, + UseAsMetaReport: "true", + UseAsConfigCenter: "false", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "true", result.UseAsMetaReport) + assert.Equal(t, "false", result.UseAsConfigCenter) +} + +// Test compatRegistryConfig with numeric fields +func TestCompatRegistryConfigNumeric(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Weight: 500, + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + // Verify weight is precisely preserved from original input + assert.Equal(t, int64(500), result.Weight) +} + +// Test compatRegistryConfig with params +func TestCompatRegistryConfigWithParams(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Params: map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }, + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, 3, len(result.Params)) + assert.Equal(t, "value1", result.Params["key1"]) + assert.Equal(t, "value2", result.Params["key2"]) + assert.Equal(t, "value3", result.Params["key3"]) +} + +// Test compatApplicationConfig preserves all field types +func TestCompatApplicationConfigFieldPreservation(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "myapp", + Organization: "myorg", + Module: "mymodule", + Group: "mygroup", + Version: "2.0.0", + Owner: "owner", + Environment: "production", + MetadataType: "local", + Tag: "beta", + MetadataServicePort: "25555", + MetadataServiceProtocol: "grpc", + } + + result := compatApplicationConfig(appCfg) + + // Verify type conversion doesn't lose data + assert.IsType(t, (*config.ApplicationConfig)(nil), result) + assert.Equal(t, appCfg.Name, result.Name) + assert.Equal(t, appCfg.Organization, result.Organization) + assert.Equal(t, appCfg.Module, result.Module) +} + +// Test compatRegistryConfig preserves all field types +func TestCompatRegistryConfigFieldPreservation(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "consul", + Timeout: "30s", + Group: "consul-group", + Namespace: "consul-ns", + TTL: "120s", + Address: "localhost:8500", + Username: "consul-user", + Password: "consul-pass", + Simplified: true, + Preferred: false, + Zone: "zone-a", + Weight: 200, + Params: map[string]string{"consul-key": "consul-val"}, + RegistryType: "interface", + UseAsMetaReport: "true", + UseAsConfigCenter: "false", + } + + result := compatRegistryConfig(regCfg) + + // Verify type conversion doesn't lose data + assert.IsType(t, (*config.RegistryConfig)(nil), result) + assert.Equal(t, regCfg.Protocol, result.Protocol) + assert.Equal(t, regCfg.Timeout, result.Timeout) + assert.Equal(t, regCfg.Group, result.Group) +} + +// Test multiple conversions don't affect original +func TestCompatConfigsPreservesOriginal(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "original-app", + Version: "1.0.0", + } + + result1 := compatApplicationConfig(appCfg) + result2 := compatApplicationConfig(appCfg) + + // Original should not be modified + assert.Equal(t, "original-app", appCfg.Name) + assert.Equal(t, "1.0.0", appCfg.Version) + + // Results should have same values Review Comment: The comment "Results should have same values" is redundant and doesn't add meaningful documentation value. The assertions themselves are self-explanatory. This comment should be removed to improve code clarity. ```suggestion ``` ########## server/action_test.go: ########## @@ -0,0 +1,705 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package server + +import ( + "testing" +) + +import ( + "github.com/stretchr/testify/assert" + + "go.uber.org/atomic" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/common" + "dubbo.apache.org/dubbo-go/v3/common/constant" + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/global" + "dubbo.apache.org/dubbo-go/v3/protocol/base" +) + +// Test Prefix method +func TestPrefix(t *testing.T) { + svcOpts := &ServiceOptions{ + Id: "com.example.TestService", + } + + prefix := svcOpts.Prefix() + assert.Equal(t, "dubbo.service.com.example.TestService", prefix) +} + +// Test InitExported +func TestInitExported(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(true), + } + + svcOpts.InitExported() + assert.False(t, svcOpts.exported.Load()) +} + +// Test IsExport returns false initially +func TestIsExportFalse(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(false), + } + + assert.False(t, svcOpts.IsExport()) +} + +// Test IsExport returns true when exported +func TestIsExportTrue(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(true), + } + + assert.True(t, svcOpts.IsExport()) +} + +// Test check with valid TpsLimiter +func TestCheckValidTpsLimiter(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimiter: "", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with valid TpsLimitRate +func TestCheckValidTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "1000", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with invalid TpsLimitRate +func TestCheckInvalidTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "invalid", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with negative TpsLimitRate +func TestCheckNegativeTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "-1", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with valid TpsLimitInterval +func TestCheckValidTpsLimitInterval(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitInterval: "1000", + }, + } + + err := svcOpts.check() Review Comment: The comment states "Verify weight is precisely preserved from original input" but this is redundant with the test itself and doesn't add meaningful documentation value. This comment appears in multiple test cases (lines 136-137, 210-211) and should be removed to improve code clarity. ########## server/compat_test.go: ########## @@ -0,0 +1,328 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package server + +import ( + "testing" +) + +import ( + "github.com/stretchr/testify/assert" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test compatApplicationConfig with all fields +func TestCompatApplicationConfigAll(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Organization: "test-org", + Name: "test-app", + Module: "test-module", + Group: "test-group", + Version: "1.0.0", + Owner: "test-owner", + Environment: "test-env", + MetadataType: "remote", + Tag: "v1", + MetadataServicePort: "30880", + MetadataServiceProtocol: "triple", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-org", result.Organization) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "test-module", result.Module) + assert.Equal(t, "test-group", result.Group) + assert.Equal(t, "1.0.0", result.Version) + assert.Equal(t, "test-owner", result.Owner) + assert.Equal(t, "test-env", result.Environment) + assert.Equal(t, "remote", result.MetadataType) + assert.Equal(t, "v1", result.Tag) + assert.Equal(t, "30880", result.MetadataServicePort) + assert.Equal(t, "triple", result.MetadataServiceProtocol) +} + +// Test compatApplicationConfig with partial fields +func TestCompatApplicationConfigPartial(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + Version: "1.0.0", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "1.0.0", result.Version) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) +} + +// Test compatApplicationConfig with empty config +func TestCompatApplicationConfigEmpty(t *testing.T) { + appCfg := &global.ApplicationConfig{} + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Name) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) + assert.Empty(t, result.Group) + assert.Empty(t, result.Version) + assert.Empty(t, result.Owner) + assert.Empty(t, result.Environment) + assert.Empty(t, result.MetadataType) + assert.Empty(t, result.Tag) + assert.Empty(t, result.MetadataServicePort) + assert.Empty(t, result.MetadataServiceProtocol) +} + +// Test compatRegistryConfig with all fields +func TestCompatRegistryConfigAll(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Timeout: "10s", + Group: "registry-group", + Namespace: "test-ns", + TTL: "60s", + Address: "localhost:2181", + Username: "admin", + Password: "password", + Simplified: true, + Preferred: true, + Zone: "zone1", + Weight: 100, + Params: map[string]string{"key": "value"}, + RegistryType: "service_discovery", + UseAsMetaReport: "true", + UseAsConfigCenter: "true", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + assert.Equal(t, "10s", result.Timeout) + assert.Equal(t, "registry-group", result.Group) + assert.Equal(t, "test-ns", result.Namespace) + assert.Equal(t, "60s", result.TTL) + assert.Equal(t, "localhost:2181", result.Address) + assert.Equal(t, "admin", result.Username) + assert.Equal(t, "password", result.Password) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "zone1", result.Zone) + // Verify weight is precisely preserved from original input + assert.Equal(t, int64(100), result.Weight) + assert.Equal(t, "service_discovery", result.RegistryType) + assert.Equal(t, "true", result.UseAsMetaReport) + assert.Equal(t, "true", result.UseAsConfigCenter) + assert.NotNil(t, result.Params) + assert.Equal(t, "value", result.Params["key"]) +} + +// Test compatRegistryConfig with partial fields +func TestCompatRegistryConfigPartial(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "nacos", + Address: "localhost:8848", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "nacos", result.Protocol) + assert.Equal(t, "localhost:8848", result.Address) + assert.Empty(t, result.Timeout) + assert.Empty(t, result.Group) +} + +// Test compatRegistryConfig with empty config +func TestCompatRegistryConfigEmpty(t *testing.T) { + regCfg := &global.RegistryConfig{} + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Protocol) + assert.Empty(t, result.Address) + assert.Empty(t, result.Timeout) + assert.Empty(t, result.Group) + assert.Empty(t, result.Namespace) + assert.Empty(t, result.TTL) + assert.Empty(t, result.Username) + assert.Empty(t, result.Password) + assert.False(t, result.Simplified) + assert.False(t, result.Preferred) +} + +// Test compatRegistryConfig with boolean fields +func TestCompatRegistryConfigBooleans(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "etcd", + Simplified: true, + Preferred: true, + UseAsMetaReport: "true", + UseAsConfigCenter: "false", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "true", result.UseAsMetaReport) + assert.Equal(t, "false", result.UseAsConfigCenter) +} + +// Test compatRegistryConfig with numeric fields +func TestCompatRegistryConfigNumeric(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Weight: 500, + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + // Verify weight is precisely preserved from original input + assert.Equal(t, int64(500), result.Weight) +} + +// Test compatRegistryConfig with params +func TestCompatRegistryConfigWithParams(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Params: map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }, + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, 3, len(result.Params)) + assert.Equal(t, "value1", result.Params["key1"]) + assert.Equal(t, "value2", result.Params["key2"]) + assert.Equal(t, "value3", result.Params["key3"]) +} + +// Test compatApplicationConfig preserves all field types +func TestCompatApplicationConfigFieldPreservation(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "myapp", + Organization: "myorg", + Module: "mymodule", + Group: "mygroup", + Version: "2.0.0", + Owner: "owner", + Environment: "production", + MetadataType: "local", + Tag: "beta", + MetadataServicePort: "25555", + MetadataServiceProtocol: "grpc", + } + + result := compatApplicationConfig(appCfg) + + // Verify type conversion doesn't lose data + assert.IsType(t, (*config.ApplicationConfig)(nil), result) + assert.Equal(t, appCfg.Name, result.Name) + assert.Equal(t, appCfg.Organization, result.Organization) + assert.Equal(t, appCfg.Module, result.Module) +} + +// Test compatRegistryConfig preserves all field types +func TestCompatRegistryConfigFieldPreservation(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "consul", + Timeout: "30s", + Group: "consul-group", + Namespace: "consul-ns", + TTL: "120s", + Address: "localhost:8500", + Username: "consul-user", + Password: "consul-pass", + Simplified: true, + Preferred: false, + Zone: "zone-a", + Weight: 200, + Params: map[string]string{"consul-key": "consul-val"}, + RegistryType: "interface", + UseAsMetaReport: "true", + UseAsConfigCenter: "false", + } + + result := compatRegistryConfig(regCfg) + + // Verify type conversion doesn't lose data + assert.IsType(t, (*config.RegistryConfig)(nil), result) + assert.Equal(t, regCfg.Protocol, result.Protocol) + assert.Equal(t, regCfg.Timeout, result.Timeout) + assert.Equal(t, regCfg.Group, result.Group) +} + +// Test multiple conversions don't affect original +func TestCompatConfigsPreservesOriginal(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "original-app", + Version: "1.0.0", + } + + result1 := compatApplicationConfig(appCfg) + result2 := compatApplicationConfig(appCfg) + + // Original should not be modified Review Comment: The comment "Original should not be modified" is redundant and doesn't add meaningful documentation value. The assertion itself is self-explanatory. This comment should be removed to improve code clarity. ```suggestion ``` ########## server/action_test.go: ########## @@ -0,0 +1,705 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package server + +import ( + "testing" +) + +import ( + "github.com/stretchr/testify/assert" + + "go.uber.org/atomic" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/common" + "dubbo.apache.org/dubbo-go/v3/common/constant" + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/global" + "dubbo.apache.org/dubbo-go/v3/protocol/base" +) + +// Test Prefix method +func TestPrefix(t *testing.T) { + svcOpts := &ServiceOptions{ + Id: "com.example.TestService", + } + + prefix := svcOpts.Prefix() + assert.Equal(t, "dubbo.service.com.example.TestService", prefix) +} + +// Test InitExported +func TestInitExported(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(true), + } + + svcOpts.InitExported() + assert.False(t, svcOpts.exported.Load()) +} + +// Test IsExport returns false initially +func TestIsExportFalse(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(false), + } + + assert.False(t, svcOpts.IsExport()) +} + +// Test IsExport returns true when exported +func TestIsExportTrue(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(true), + } + + assert.True(t, svcOpts.IsExport()) +} + +// Test check with valid TpsLimiter +func TestCheckValidTpsLimiter(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimiter: "", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with valid TpsLimitRate +func TestCheckValidTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "1000", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with invalid TpsLimitRate +func TestCheckInvalidTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "invalid", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with negative TpsLimitRate +func TestCheckNegativeTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "-1", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with valid TpsLimitInterval +func TestCheckValidTpsLimitInterval(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitInterval: "1000", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with invalid TpsLimitInterval +func TestCheckInvalidTpsLimitInterval(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitInterval: "invalid", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with negative TpsLimitInterval +func TestCheckNegativeTpsLimitInterval(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitInterval: "-1", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test Implement +func TestImplement(t *testing.T) { + svcOpts := &ServiceOptions{} + mockService := &mockRPCService{} + + svcOpts.Implement(mockService) + + assert.Equal(t, mockService, svcOpts.rpcService) +} + +// Test Unexport when not exported +func TestUnexportNotExported(t *testing.T) { + svcOpts := &ServiceOptions{ + unexported: atomic.NewBool(false), + exported: atomic.NewBool(false), + exporters: []base.Exporter{}, + } + + svcOpts.Unexport() + assert.False(t, svcOpts.exported.Load()) + assert.False(t, svcOpts.unexported.Load()) +} + +// Test Unexport when already unexported +func TestUnexportAlreadyUnexported(t *testing.T) { + svcOpts := &ServiceOptions{ + unexported: atomic.NewBool(true), + exported: atomic.NewBool(false), + exporters: []base.Exporter{}, + } + + svcOpts.Unexport() + assert.True(t, svcOpts.unexported.Load()) +} + +// Test removeDuplicateElement +func TestRemoveDuplicateElement(t *testing.T) { + items := []string{"a", "b", "a", "c", "b", ""} + result := removeDuplicateElement(items) + + assert.Len(t, result, 3) + assert.Contains(t, result, "a") + assert.Contains(t, result, "b") + assert.Contains(t, result, "c") +} Review Comment: The comment states "Verify weight is precisely preserved from original input" but this is redundant with the test itself and doesn't add meaningful documentation value. This comment should be removed to improve code clarity. ########## server/action_test.go: ########## @@ -0,0 +1,705 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package server + +import ( + "testing" +) + +import ( + "github.com/stretchr/testify/assert" + + "go.uber.org/atomic" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/common" + "dubbo.apache.org/dubbo-go/v3/common/constant" + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/global" + "dubbo.apache.org/dubbo-go/v3/protocol/base" +) + +// Test Prefix method +func TestPrefix(t *testing.T) { + svcOpts := &ServiceOptions{ + Id: "com.example.TestService", + } + + prefix := svcOpts.Prefix() + assert.Equal(t, "dubbo.service.com.example.TestService", prefix) +} + +// Test InitExported +func TestInitExported(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(true), + } + + svcOpts.InitExported() + assert.False(t, svcOpts.exported.Load()) +} + +// Test IsExport returns false initially +func TestIsExportFalse(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(false), + } + + assert.False(t, svcOpts.IsExport()) +} + +// Test IsExport returns true when exported +func TestIsExportTrue(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(true), + } + + assert.True(t, svcOpts.IsExport()) +} + +// Test check with valid TpsLimiter +func TestCheckValidTpsLimiter(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimiter: "", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with valid TpsLimitRate +func TestCheckValidTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "1000", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with invalid TpsLimitRate +func TestCheckInvalidTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "invalid", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with negative TpsLimitRate +func TestCheckNegativeTpsLimitRate(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "-1", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with valid TpsLimitInterval +func TestCheckValidTpsLimitInterval(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitInterval: "1000", + }, + } + + err := svcOpts.check() + assert.NoError(t, err) +} + +// Test check with invalid TpsLimitInterval +func TestCheckInvalidTpsLimitInterval(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitInterval: "invalid", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test check with negative TpsLimitInterval +func TestCheckNegativeTpsLimitInterval(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitInterval: "-1", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test Implement +func TestImplement(t *testing.T) { + svcOpts := &ServiceOptions{} + mockService := &mockRPCService{} + + svcOpts.Implement(mockService) + + assert.Equal(t, mockService, svcOpts.rpcService) +} + +// Test Unexport when not exported +func TestUnexportNotExported(t *testing.T) { + svcOpts := &ServiceOptions{ + unexported: atomic.NewBool(false), + exported: atomic.NewBool(false), + exporters: []base.Exporter{}, + } + + svcOpts.Unexport() + assert.False(t, svcOpts.exported.Load()) + assert.False(t, svcOpts.unexported.Load()) +} + +// Test Unexport when already unexported +func TestUnexportAlreadyUnexported(t *testing.T) { + svcOpts := &ServiceOptions{ + unexported: atomic.NewBool(true), + exported: atomic.NewBool(false), + exporters: []base.Exporter{}, + } + + svcOpts.Unexport() + assert.True(t, svcOpts.unexported.Load()) +} + +// Test removeDuplicateElement +func TestRemoveDuplicateElement(t *testing.T) { + items := []string{"a", "b", "a", "c", "b", ""} + result := removeDuplicateElement(items) + + assert.Len(t, result, 3) + assert.Contains(t, result, "a") + assert.Contains(t, result, "b") + assert.Contains(t, result, "c") +} + +// Test removeDuplicateElement with empty slice +func TestRemoveDuplicateElementEmpty(t *testing.T) { + items := []string{} + result := removeDuplicateElement(items) + + assert.Len(t, result, 0) +} + +// Test removeDuplicateElement with only empty strings +func TestRemoveDuplicateElementOnlyEmpty(t *testing.T) { + items := []string{"", "", ""} + result := removeDuplicateElement(items) + + assert.Len(t, result, 0) +} + +// Test removeDuplicateElement with mixed content +func TestRemoveDuplicateElementMixed(t *testing.T) { + items := []string{"reg1", "reg2", "reg1", "", "reg3", ""} + result := removeDuplicateElement(items) + + assert.Len(t, result, 3) + assert.Contains(t, result, "reg1") + assert.Contains(t, result, "reg2") + assert.Contains(t, result, "reg3") +} + +// Test getRegistryIds +func TestGetRegistryIds(t *testing.T) { + registries := map[string]*global.RegistryConfig{ + "reg1": {Protocol: "zookeeper"}, + "reg2": {Protocol: "nacos"}, + "reg3": {Protocol: "etcd"}, + } + + ids := getRegistryIds(registries) + + assert.Len(t, ids, 3) + assert.Contains(t, ids, "reg1") + assert.Contains(t, ids, "reg2") + assert.Contains(t, ids, "reg3") +} + +// Test getRegistryIds with empty map +func TestGetRegistryIdsEmpty(t *testing.T) { + registries := map[string]*global.RegistryConfig{} + + ids := getRegistryIds(registries) + + assert.Len(t, ids, 0) +} + +// Test GetExportedUrls when not exported +func TestGetExportedUrlsNotExported(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(false), + exporters: []base.Exporter{}, + } + + urls := svcOpts.GetExportedUrls() + assert.Nil(t, urls) +} + +// Test GetExportedUrls when exported with no exporters +func TestGetExportedUrlsExportedEmpty(t *testing.T) { + svcOpts := &ServiceOptions{ + exported: atomic.NewBool(true), + exporters: []base.Exporter{}, + } + + urls := svcOpts.GetExportedUrls() + assert.Len(t, urls, 0) +} + +// Test getUrlMap basic functionality +func TestGetUrlMapBasic(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Filter: "", + Params: map[string]string{}, + NotRegister: false, + Tag: "v1", + Group: "test-group", + Version: "1.0.0", + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + Organization: "test-org", + Module: "test-module", + Owner: "test-owner", + Environment: "test-env", + Version: "1.0.0", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{Enable: nil}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{Enable: nil}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.NotNil(t, urlMap) + assert.Equal(t, "com.example.Service", urlMap.Get(constant.InterfaceKey)) + assert.Equal(t, constant.ClusterKeyFailover, urlMap.Get(constant.ClusterKey)) + assert.Equal(t, constant.LoadBalanceKeyRoundRobin, urlMap.Get(constant.LoadbalanceKey)) + assert.Equal(t, "60", urlMap.Get(constant.WarmupKey)) + assert.Equal(t, "3", urlMap.Get(constant.RetriesKey)) + assert.Equal(t, constant.JSONSerialization, urlMap.Get(constant.SerializationKey)) +} + +// Test getUrlMap with custom params +func TestGetUrlMapWithParams(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Params: map[string]string{ + "customKey": "customValue", + }, + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + Organization: "test-org", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.NotNil(t, urlMap) + assert.Equal(t, "customValue", urlMap.Get("customKey")) +} + +// Test getUrlMap with group and version +func TestGetUrlMapWithGroupAndVersion(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Group: "test-group", + Version: "1.0.0", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Params: map[string]string{}, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.Equal(t, "test-group", urlMap.Get(constant.GroupKey)) + assert.Equal(t, "1.0.0", urlMap.Get(constant.VersionKey)) +} + +// Test getUrlMap with methods +func TestGetUrlMapWithMethods(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Params: map[string]string{}, + Methods: []*global.MethodConfig{ + { + Name: "testMethod", + LoadBalance: "random", + Retries: "5", + Weight: 200, + }, + }, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + urlMap := svcOpts.getUrlMap() + assert.Equal(t, "random", urlMap.Get("methods.testMethod."+constant.LoadbalanceKey)) + assert.Equal(t, "5", urlMap.Get("methods.testMethod."+constant.RetriesKey)) + assert.Equal(t, "200", urlMap.Get("methods.testMethod."+constant.WeightKey)) +} + +// Test loadProtocol with matching IDs +func TestLoadProtocol(t *testing.T) { + protocols := map[string]*global.ProtocolConfig{ + "dubbo": { + Name: "dubbo", + Port: "20880", + }, + "triple": { + Name: "triple", + Port: "50051", + }, + } + + protocolIDs := []string{"dubbo", "triple"} + result := loadProtocol(protocolIDs, protocols) + + assert.Len(t, result, 2) +} + +// Test loadProtocol with partial matching +func TestLoadProtocolPartialMatch(t *testing.T) { + protocols := map[string]*global.ProtocolConfig{ + "dubbo": { + Name: "dubbo", + Port: "20880", + }, + "triple": { + Name: "triple", + Port: "50051", + }, + } + + protocolIDs := []string{"dubbo"} + result := loadProtocol(protocolIDs, protocols) + + assert.Len(t, result, 1) + assert.Equal(t, "dubbo", result[0].Name) +} + +// Test loadProtocol with no matching IDs +func TestLoadProtocolNoMatch(t *testing.T) { + protocols := map[string]*global.ProtocolConfig{ + "dubbo": { + Name: "dubbo", + Port: "20880", + }, + } + + protocolIDs := []string{"non-existent"} + result := loadProtocol(protocolIDs, protocols) + + assert.Len(t, result, 0) +} + +// Test setRegistrySubURL +func TestSetRegistrySubURL(t *testing.T) { + ivkURL, _ := common.NewURL("dubbo://localhost:20880/com.example.Service") + ivkURL.AddParam(constant.RegistryKey, "zookeeper") + ivkURL.AddParam(constant.RegistryTypeKey, "service_discovery") + + regURL, _ := common.NewURL("registry://zookeeper:2181") + regURL.AddParam(constant.RegistryKey, "zookeeper") + regURL.AddParam(constant.RegistryTypeKey, "service_discovery") + + setRegistrySubURL(ivkURL, regURL) + + assert.Equal(t, "zookeeper", ivkURL.GetParam(constant.RegistryKey, "")) + assert.Equal(t, "service_discovery", ivkURL.GetParam(constant.RegistryTypeKey, "")) + assert.NotNil(t, regURL.SubURL) +} + +// Test Unexport when exported +func TestUnexportWhenExported(t *testing.T) { + svcOpts := &ServiceOptions{ + unexported: atomic.NewBool(false), + exported: atomic.NewBool(true), + exporters: []base.Exporter{}, + } + + svcOpts.Unexport() + assert.False(t, svcOpts.exported.Load()) + assert.True(t, svcOpts.unexported.Load()) +} + +// Test check with multiple validation errors +func TestCheckMultipleErrors(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + TpsLimitRate: "invalid", + TpsLimitInterval: "also-invalid", + }, + } + + err := svcOpts.check() + assert.Error(t, err) +} + +// Test getUrlMap with tag +func TestGetUrlMapWithTag(t *testing.T) { + svcOpts := &ServiceOptions{ + Service: &global.ServiceConfig{ + Interface: "com.example.Service", + Tag: "production", + Cluster: constant.ClusterKeyFailover, + Loadbalance: constant.LoadBalanceKeyRoundRobin, + Warmup: "60", + Retries: "3", + Serialization: constant.JSONSerialization, + Params: map[string]string{}, + }, + Provider: &global.ProviderConfig{}, + applicationCompat: &config.ApplicationConfig{ + Name: "test-app", + }, + srvOpts: &ServerOptions{ + Metrics: &global.MetricsConfig{}, + Otel: &global.OtelConfig{ + TracingConfig: &global.OtelTraceConfig{}, + }, + }, + } + + // Tag is added separately in Export method, not in getUrlMap + assert.Equal(t, "production", svcOpts.Service.Tag) +} + Review Comment: The comment "Tag is added separately in Export method, not in getUrlMap" is misleading and doesn't test anything meaningful. This test only asserts that the Tag field is set, but doesn't verify the actual behavior related to URL mapping. Consider either removing this test or making it test the actual behavior being documented. ```suggestion ``` ########## server/compat_test.go: ########## @@ -0,0 +1,328 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package server + +import ( + "testing" +) + +import ( + "github.com/stretchr/testify/assert" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test compatApplicationConfig with all fields +func TestCompatApplicationConfigAll(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Organization: "test-org", + Name: "test-app", + Module: "test-module", + Group: "test-group", + Version: "1.0.0", + Owner: "test-owner", + Environment: "test-env", + MetadataType: "remote", + Tag: "v1", + MetadataServicePort: "30880", + MetadataServiceProtocol: "triple", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-org", result.Organization) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "test-module", result.Module) + assert.Equal(t, "test-group", result.Group) + assert.Equal(t, "1.0.0", result.Version) + assert.Equal(t, "test-owner", result.Owner) + assert.Equal(t, "test-env", result.Environment) + assert.Equal(t, "remote", result.MetadataType) + assert.Equal(t, "v1", result.Tag) + assert.Equal(t, "30880", result.MetadataServicePort) + assert.Equal(t, "triple", result.MetadataServiceProtocol) +} + +// Test compatApplicationConfig with partial fields +func TestCompatApplicationConfigPartial(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + Version: "1.0.0", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "1.0.0", result.Version) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) +} + +// Test compatApplicationConfig with empty config +func TestCompatApplicationConfigEmpty(t *testing.T) { + appCfg := &global.ApplicationConfig{} + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Name) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) + assert.Empty(t, result.Group) + assert.Empty(t, result.Version) + assert.Empty(t, result.Owner) + assert.Empty(t, result.Environment) + assert.Empty(t, result.MetadataType) + assert.Empty(t, result.Tag) + assert.Empty(t, result.MetadataServicePort) + assert.Empty(t, result.MetadataServiceProtocol) +} + +// Test compatRegistryConfig with all fields +func TestCompatRegistryConfigAll(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Timeout: "10s", + Group: "registry-group", + Namespace: "test-ns", + TTL: "60s", + Address: "localhost:2181", + Username: "admin", + Password: "password", + Simplified: true, + Preferred: true, + Zone: "zone1", + Weight: 100, + Params: map[string]string{"key": "value"}, + RegistryType: "service_discovery", + UseAsMetaReport: "true", + UseAsConfigCenter: "true", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + assert.Equal(t, "10s", result.Timeout) + assert.Equal(t, "registry-group", result.Group) + assert.Equal(t, "test-ns", result.Namespace) + assert.Equal(t, "60s", result.TTL) + assert.Equal(t, "localhost:2181", result.Address) + assert.Equal(t, "admin", result.Username) + assert.Equal(t, "password", result.Password) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "zone1", result.Zone) + // Verify weight is precisely preserved from original input + assert.Equal(t, int64(100), result.Weight) + assert.Equal(t, "service_discovery", result.RegistryType) + assert.Equal(t, "true", result.UseAsMetaReport) + assert.Equal(t, "true", result.UseAsConfigCenter) + assert.NotNil(t, result.Params) + assert.Equal(t, "value", result.Params["key"]) +} + +// Test compatRegistryConfig with partial fields +func TestCompatRegistryConfigPartial(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "nacos", + Address: "localhost:8848", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "nacos", result.Protocol) + assert.Equal(t, "localhost:8848", result.Address) + assert.Empty(t, result.Timeout) + assert.Empty(t, result.Group) +} + +// Test compatRegistryConfig with empty config +func TestCompatRegistryConfigEmpty(t *testing.T) { + regCfg := &global.RegistryConfig{} + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Protocol) + assert.Empty(t, result.Address) + assert.Empty(t, result.Timeout) + assert.Empty(t, result.Group) + assert.Empty(t, result.Namespace) + assert.Empty(t, result.TTL) + assert.Empty(t, result.Username) + assert.Empty(t, result.Password) + assert.False(t, result.Simplified) + assert.False(t, result.Preferred) +} + +// Test compatRegistryConfig with boolean fields +func TestCompatRegistryConfigBooleans(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "etcd", + Simplified: true, + Preferred: true, + UseAsMetaReport: "true", + UseAsConfigCenter: "false", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "true", result.UseAsMetaReport) + assert.Equal(t, "false", result.UseAsConfigCenter) +} + +// Test compatRegistryConfig with numeric fields +func TestCompatRegistryConfigNumeric(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Weight: 500, + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + // Verify weight is precisely preserved from original input Review Comment: The comment states "Verify weight is precisely preserved from original input" but this is redundant with the test itself and doesn't add meaningful documentation value. This comment should be removed to improve code clarity. ```suggestion ``` ########## server/compat_test.go: ########## @@ -0,0 +1,328 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package server + +import ( + "testing" +) + +import ( + "github.com/stretchr/testify/assert" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test compatApplicationConfig with all fields +func TestCompatApplicationConfigAll(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Organization: "test-org", + Name: "test-app", + Module: "test-module", + Group: "test-group", + Version: "1.0.0", + Owner: "test-owner", + Environment: "test-env", + MetadataType: "remote", + Tag: "v1", + MetadataServicePort: "30880", + MetadataServiceProtocol: "triple", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-org", result.Organization) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "test-module", result.Module) + assert.Equal(t, "test-group", result.Group) + assert.Equal(t, "1.0.0", result.Version) + assert.Equal(t, "test-owner", result.Owner) + assert.Equal(t, "test-env", result.Environment) + assert.Equal(t, "remote", result.MetadataType) + assert.Equal(t, "v1", result.Tag) + assert.Equal(t, "30880", result.MetadataServicePort) + assert.Equal(t, "triple", result.MetadataServiceProtocol) +} + +// Test compatApplicationConfig with partial fields +func TestCompatApplicationConfigPartial(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + Version: "1.0.0", + } + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Equal(t, "test-app", result.Name) + assert.Equal(t, "1.0.0", result.Version) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) +} + +// Test compatApplicationConfig with empty config +func TestCompatApplicationConfigEmpty(t *testing.T) { + appCfg := &global.ApplicationConfig{} + + result := compatApplicationConfig(appCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Name) + assert.Empty(t, result.Organization) + assert.Empty(t, result.Module) + assert.Empty(t, result.Group) + assert.Empty(t, result.Version) + assert.Empty(t, result.Owner) + assert.Empty(t, result.Environment) + assert.Empty(t, result.MetadataType) + assert.Empty(t, result.Tag) + assert.Empty(t, result.MetadataServicePort) + assert.Empty(t, result.MetadataServiceProtocol) +} + +// Test compatRegistryConfig with all fields +func TestCompatRegistryConfigAll(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Timeout: "10s", + Group: "registry-group", + Namespace: "test-ns", + TTL: "60s", + Address: "localhost:2181", + Username: "admin", + Password: "password", + Simplified: true, + Preferred: true, + Zone: "zone1", + Weight: 100, + Params: map[string]string{"key": "value"}, + RegistryType: "service_discovery", + UseAsMetaReport: "true", + UseAsConfigCenter: "true", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + assert.Equal(t, "10s", result.Timeout) + assert.Equal(t, "registry-group", result.Group) + assert.Equal(t, "test-ns", result.Namespace) + assert.Equal(t, "60s", result.TTL) + assert.Equal(t, "localhost:2181", result.Address) + assert.Equal(t, "admin", result.Username) + assert.Equal(t, "password", result.Password) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "zone1", result.Zone) + // Verify weight is precisely preserved from original input + assert.Equal(t, int64(100), result.Weight) + assert.Equal(t, "service_discovery", result.RegistryType) + assert.Equal(t, "true", result.UseAsMetaReport) + assert.Equal(t, "true", result.UseAsConfigCenter) + assert.NotNil(t, result.Params) + assert.Equal(t, "value", result.Params["key"]) +} + +// Test compatRegistryConfig with partial fields +func TestCompatRegistryConfigPartial(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "nacos", + Address: "localhost:8848", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "nacos", result.Protocol) + assert.Equal(t, "localhost:8848", result.Address) + assert.Empty(t, result.Timeout) + assert.Empty(t, result.Group) +} + +// Test compatRegistryConfig with empty config +func TestCompatRegistryConfigEmpty(t *testing.T) { + regCfg := &global.RegistryConfig{} + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Empty(t, result.Protocol) + assert.Empty(t, result.Address) + assert.Empty(t, result.Timeout) + assert.Empty(t, result.Group) + assert.Empty(t, result.Namespace) + assert.Empty(t, result.TTL) + assert.Empty(t, result.Username) + assert.Empty(t, result.Password) + assert.False(t, result.Simplified) + assert.False(t, result.Preferred) +} + +// Test compatRegistryConfig with boolean fields +func TestCompatRegistryConfigBooleans(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "etcd", + Simplified: true, + Preferred: true, + UseAsMetaReport: "true", + UseAsConfigCenter: "false", + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.True(t, result.Simplified) + assert.True(t, result.Preferred) + assert.Equal(t, "true", result.UseAsMetaReport) + assert.Equal(t, "false", result.UseAsConfigCenter) +} + +// Test compatRegistryConfig with numeric fields +func TestCompatRegistryConfigNumeric(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Weight: 500, + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, "zookeeper", result.Protocol) + // Verify weight is precisely preserved from original input + assert.Equal(t, int64(500), result.Weight) +} + +// Test compatRegistryConfig with params +func TestCompatRegistryConfigWithParams(t *testing.T) { + regCfg := &global.RegistryConfig{ + Protocol: "zookeeper", + Params: map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }, + } + + result := compatRegistryConfig(regCfg) + + assert.NotNil(t, result) + assert.Equal(t, 3, len(result.Params)) + assert.Equal(t, "value1", result.Params["key1"]) + assert.Equal(t, "value2", result.Params["key2"]) + assert.Equal(t, "value3", result.Params["key3"]) +} + +// Test compatApplicationConfig preserves all field types +func TestCompatApplicationConfigFieldPreservation(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "myapp", + Organization: "myorg", + Module: "mymodule", + Group: "mygroup", + Version: "2.0.0", + Owner: "owner", + Environment: "production", + MetadataType: "local", + Tag: "beta", + MetadataServicePort: "25555", + MetadataServiceProtocol: "grpc", + } + + result := compatApplicationConfig(appCfg) + + // Verify type conversion doesn't lose data Review Comment: The comment states "Verify type conversion doesn't lose data" but this is redundant and doesn't add meaningful documentation value. The test code itself is self-explanatory. This comment should be removed to improve code clarity. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
