Copilot commented on code in PR #3129: URL: https://github.com/apache/dubbo-go/pull/3129#discussion_r2637692335
########## server/server_test.go: ########## @@ -0,0 +1,417 @@ +/* + * 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/common" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test NewServer creates a server successfully +func TestNewServer(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.NotNil(t, srv.cfg) + assert.NotNil(t, srv.svcOptsMap) + assert.NotNil(t, srv.interfaceNameServices) +} + +// Test NewServer with options +func TestNewServerWithOptions(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + } + srv, err := NewServer( + SetServerApplication(appCfg), + WithServerGroup("test-group"), + ) + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.Equal(t, appCfg, srv.cfg.Application) + assert.Equal(t, "test-group", srv.cfg.Provider.Group) +} + +// Test GetServiceOptions +func TestGetServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptions("test-service") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptions returns nil for non-existent service +func TestGetServiceOptionsNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptions("non-existent") + assert.Nil(t, retrieved) +} + +// Test GetServiceInfo +func TestGetServiceInfo(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.info = &common.ServiceInfo{ + InterfaceName: "com.example.TestService", + } + srv.registerServiceOptions(svcOpts) + + info := srv.GetServiceInfo("test-service") + assert.NotNil(t, info) + assert.Equal(t, "com.example.TestService", info.InterfaceName) +} + +// Test GetServiceInfo returns nil for non-existent service +func TestGetServiceInfoNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + info := srv.GetServiceInfo("non-existent") + assert.Nil(t, info) +} + +// Test GetRPCService +func TestGetRPCService(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + mockService := &mockRPCService{} + svcOpts.rpcService = mockService + srv.registerServiceOptions(svcOpts) + + rpcService := srv.GetRPCService("test-service") + assert.NotNil(t, rpcService) + assert.Equal(t, mockService, rpcService) +} + +// Test GetRPCService returns nil for non-existent service +func TestGetRPCServiceNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + rpcService := srv.GetRPCService("non-existent") + assert.Nil(t, rpcService) +} + +// Test GetServiceOptionsByInterfaceName +func TestGetServiceOptionsByInterfaceName(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptionsByInterfaceName("com.example.TestService") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptionsByInterfaceName returns nil for non-existent interface +func TestGetServiceOptionsByInterfaceNameNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptionsByInterfaceName("non.existent.Service") + assert.Nil(t, retrieved) +} + +// Test registerServiceOptions +func TestRegisterServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + + srv.registerServiceOptions(svcOpts) + + // Verify via maps + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Equal(t, svcOpts, srv.interfaceNameServices["com.example.TestService"]) +} + +// Test registerServiceOptions with empty interface +func TestRegisterServiceOptionsEmptyInterface(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "" + + srv.registerServiceOptions(svcOpts) + + // Should be in svcOptsMap but not in interfaceNameServices + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Nil(t, srv.interfaceNameServices[""]) +} + +// Test SetProviderServices +func TestSetProviderServices(t *testing.T) { + // Reset the internal services slice + internalProServices = make([]*InternalService, 0, 16) + + internalService := &InternalService{ + Name: "test-internal-service", + Priority: 0, + Init: func(options *ServiceOptions) (*ServiceDefinition, bool) { + return &ServiceDefinition{ + Handler: &mockRPCService{}, + Info: nil, + Opts: []ServiceOption{}, + }, true + }, + } + + SetProviderServices(internalService) + + assert.Len(t, internalProServices, 1) + assert.Equal(t, "test-internal-service", internalProServices[0].Name) +} + +// Test SetProviderServices with empty name +func TestSetProviderServicesEmptyName(t *testing.T) { + // Reset the internal services slice + internalProServices = make([]*InternalService, 0, 16) + + internalService := &InternalService{ + Name: "", + } + + SetProviderServices(internalService) + + assert.Len(t, internalProServices, 0) +} + +// Test enhanceServiceInfo with nil info +func TestEnhanceServiceInfoNil(t *testing.T) { + info := enhanceServiceInfo(nil) + assert.Nil(t, info) +} + +// Test enhanceServiceInfo with methods +func TestEnhanceServiceInfo(t *testing.T) { + info := &common.ServiceInfo{ + InterfaceName: "com.example.Service", + Methods: []common.MethodInfo{ + { + Name: "sayHello", + }, + }, + } + + result := enhanceServiceInfo(info) + assert.NotNil(t, result) + // Should have doubled methods (original + case-swapped) + assert.Equal(t, 2, len(result.Methods)) + assert.Equal(t, "sayHello", result.Methods[0].Name) + // The swapped version should have capitalized first letter + assert.NotEqual(t, "sayHello", result.Methods[1].Name) +} + +// Test getMetadataPort with default protocol +func TestGetMetadataPortWithDefaultProtocol(t *testing.T) { + opts := defaultServerOptions() + opts.Application.MetadataServicePort = "" + opts.Protocols = map[string]*global.ProtocolConfig{ + "dubbo": { + Port: "20880", + }, + } + + port := getMetadataPort(opts) + assert.Equal(t, 20880, port) +} + +// Test getMetadataPort with explicit port +func TestGetMetadataPortExplicit(t *testing.T) { + opts := defaultServerOptions() + opts.Application.MetadataServicePort = "30880" + + port := getMetadataPort(opts) + assert.Equal(t, 30880, port) +} + +// Test getMetadataPort with no port +func TestGetMetadataPortNoPort(t *testing.T) { + opts := defaultServerOptions() + opts.Application.MetadataServicePort = "" + opts.Protocols = map[string]*global.ProtocolConfig{} + + port := getMetadataPort(opts) + assert.Equal(t, 0, port) +} + +// Test getMetadataPort with invalid port +func TestGetMetadataPortInvalid(t *testing.T) { + opts := defaultServerOptions() + opts.Application.MetadataServicePort = "invalid" + + port := getMetadataPort(opts) + assert.Equal(t, 0, port) +} + +// Mock RPCService for testing +type mockRPCService struct{} + +func (m *mockRPCService) Invoke(methodName string, params []any, results []any) error { + return nil +} + +func (m *mockRPCService) Reference() string { + return "com.example.MockService" +} + +// Test concurrency: multiple goroutines registering services +func TestConcurrentServiceRegistration(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + done := make(chan bool) + for i := 0; i < 10; i++ { + go func(idx int) { + svcOpts := defaultServiceOptions() + svcOpts.Id = "service-" + string(rune(idx)) + srv.registerServiceOptions(svcOpts) + done <- true + }(i) + } + + for i := 0; i < 10; i++ { + <-done + } + + // Verify all services were registered + assert.Equal(t, 10, len(srv.svcOptsMap)) Review Comment: Direct access to `srv.svcOptsMap` bypasses the mutex protection provided by the Server struct. The `svcOptsMap` field is protected by `srv.mu` and should only be accessed through methods like `GetServiceOptions` or while holding the appropriate lock. This creates a data race condition where the test reads the map while goroutines may still be writing to it. Consider using a synchronization mechanism like `sync.WaitGroup` to ensure all goroutines complete before reading, and access the map size through a safe method or use the count of successful registrations. ########## server/server_test.go: ########## @@ -0,0 +1,417 @@ +/* + * 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/common" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test NewServer creates a server successfully +func TestNewServer(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.NotNil(t, srv.cfg) + assert.NotNil(t, srv.svcOptsMap) + assert.NotNil(t, srv.interfaceNameServices) +} + +// Test NewServer with options +func TestNewServerWithOptions(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + } + srv, err := NewServer( + SetServerApplication(appCfg), + WithServerGroup("test-group"), + ) + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.Equal(t, appCfg, srv.cfg.Application) + assert.Equal(t, "test-group", srv.cfg.Provider.Group) +} + +// Test GetServiceOptions +func TestGetServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptions("test-service") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptions returns nil for non-existent service +func TestGetServiceOptionsNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptions("non-existent") + assert.Nil(t, retrieved) +} + +// Test GetServiceInfo +func TestGetServiceInfo(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.info = &common.ServiceInfo{ + InterfaceName: "com.example.TestService", + } + srv.registerServiceOptions(svcOpts) + + info := srv.GetServiceInfo("test-service") + assert.NotNil(t, info) + assert.Equal(t, "com.example.TestService", info.InterfaceName) +} + +// Test GetServiceInfo returns nil for non-existent service +func TestGetServiceInfoNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + info := srv.GetServiceInfo("non-existent") + assert.Nil(t, info) +} + +// Test GetRPCService +func TestGetRPCService(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + mockService := &mockRPCService{} + svcOpts.rpcService = mockService + srv.registerServiceOptions(svcOpts) + + rpcService := srv.GetRPCService("test-service") + assert.NotNil(t, rpcService) + assert.Equal(t, mockService, rpcService) +} + +// Test GetRPCService returns nil for non-existent service +func TestGetRPCServiceNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + rpcService := srv.GetRPCService("non-existent") + assert.Nil(t, rpcService) +} + +// Test GetServiceOptionsByInterfaceName +func TestGetServiceOptionsByInterfaceName(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptionsByInterfaceName("com.example.TestService") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptionsByInterfaceName returns nil for non-existent interface +func TestGetServiceOptionsByInterfaceNameNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptionsByInterfaceName("non.existent.Service") + assert.Nil(t, retrieved) +} + +// Test registerServiceOptions +func TestRegisterServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + + srv.registerServiceOptions(svcOpts) + + // Verify via maps + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Equal(t, svcOpts, srv.interfaceNameServices["com.example.TestService"]) +} + +// Test registerServiceOptions with empty interface +func TestRegisterServiceOptionsEmptyInterface(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "" + + srv.registerServiceOptions(svcOpts) + + // Should be in svcOptsMap but not in interfaceNameServices + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Nil(t, srv.interfaceNameServices[""]) +} + +// Test SetProviderServices +func TestSetProviderServices(t *testing.T) { + // Reset the internal services slice + internalProServices = make([]*InternalService, 0, 16) + + internalService := &InternalService{ + Name: "test-internal-service", + Priority: 0, + Init: func(options *ServiceOptions) (*ServiceDefinition, bool) { + return &ServiceDefinition{ + Handler: &mockRPCService{}, + Info: nil, + Opts: []ServiceOption{}, + }, true + }, + } + + SetProviderServices(internalService) + + assert.Len(t, internalProServices, 1) + assert.Equal(t, "test-internal-service", internalProServices[0].Name) +} + +// Test SetProviderServices with empty name +func TestSetProviderServicesEmptyName(t *testing.T) { + // Reset the internal services slice + internalProServices = make([]*InternalService, 0, 16) + + internalService := &InternalService{ + Name: "", + } + + SetProviderServices(internalService) + + assert.Len(t, internalProServices, 0) +} + +// Test enhanceServiceInfo with nil info +func TestEnhanceServiceInfoNil(t *testing.T) { + info := enhanceServiceInfo(nil) + assert.Nil(t, info) +} + +// Test enhanceServiceInfo with methods +func TestEnhanceServiceInfo(t *testing.T) { + info := &common.ServiceInfo{ + InterfaceName: "com.example.Service", + Methods: []common.MethodInfo{ + { + Name: "sayHello", + }, + }, + } + + result := enhanceServiceInfo(info) + assert.NotNil(t, result) + // Should have doubled methods (original + case-swapped) + assert.Equal(t, 2, len(result.Methods)) + assert.Equal(t, "sayHello", result.Methods[0].Name) + // The swapped version should have capitalized first letter + assert.NotEqual(t, "sayHello", result.Methods[1].Name) +} + +// Test getMetadataPort with default protocol +func TestGetMetadataPortWithDefaultProtocol(t *testing.T) { + opts := defaultServerOptions() + opts.Application.MetadataServicePort = "" + opts.Protocols = map[string]*global.ProtocolConfig{ + "dubbo": { + Port: "20880", + }, + } + + port := getMetadataPort(opts) + assert.Equal(t, 20880, port) +} + +// Test getMetadataPort with explicit port +func TestGetMetadataPortExplicit(t *testing.T) { + opts := defaultServerOptions() + opts.Application.MetadataServicePort = "30880" + + port := getMetadataPort(opts) + assert.Equal(t, 30880, port) +} + +// Test getMetadataPort with no port +func TestGetMetadataPortNoPort(t *testing.T) { + opts := defaultServerOptions() + opts.Application.MetadataServicePort = "" + opts.Protocols = map[string]*global.ProtocolConfig{} + + port := getMetadataPort(opts) + assert.Equal(t, 0, port) +} + +// Test getMetadataPort with invalid port +func TestGetMetadataPortInvalid(t *testing.T) { + opts := defaultServerOptions() + opts.Application.MetadataServicePort = "invalid" + + port := getMetadataPort(opts) + assert.Equal(t, 0, port) +} + +// Mock RPCService for testing +type mockRPCService struct{} + +func (m *mockRPCService) Invoke(methodName string, params []any, results []any) error { + return nil +} + +func (m *mockRPCService) Reference() string { + return "com.example.MockService" +} + +// Test concurrency: multiple goroutines registering services +func TestConcurrentServiceRegistration(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + done := make(chan bool) + for i := 0; i < 10; i++ { + go func(idx int) { + svcOpts := defaultServiceOptions() + svcOpts.Id = "service-" + string(rune(idx)) Review Comment: The conversion `string(rune(idx))` does not convert integers to their string representation correctly. This converts the integer to a Unicode character instead of a decimal string. For example, `string(rune(0))` produces "\x00" not "0", and `string(rune(1))` produces "\x01" not "1". This will cause all goroutines to register services with non-printable or unexpected ID values, making the test assertion on line 324 pass for the wrong reasons. Use `strconv.Itoa(idx)` instead to properly convert the integer to its decimal string representation. ########## server/server_test.go: ########## @@ -0,0 +1,417 @@ +/* + * 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/common" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test NewServer creates a server successfully +func TestNewServer(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.NotNil(t, srv.cfg) + assert.NotNil(t, srv.svcOptsMap) + assert.NotNil(t, srv.interfaceNameServices) +} + +// Test NewServer with options +func TestNewServerWithOptions(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + } + srv, err := NewServer( + SetServerApplication(appCfg), + WithServerGroup("test-group"), + ) + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.Equal(t, appCfg, srv.cfg.Application) + assert.Equal(t, "test-group", srv.cfg.Provider.Group) +} + +// Test GetServiceOptions +func TestGetServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptions("test-service") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptions returns nil for non-existent service +func TestGetServiceOptionsNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptions("non-existent") + assert.Nil(t, retrieved) +} + +// Test GetServiceInfo +func TestGetServiceInfo(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.info = &common.ServiceInfo{ + InterfaceName: "com.example.TestService", + } + srv.registerServiceOptions(svcOpts) + + info := srv.GetServiceInfo("test-service") + assert.NotNil(t, info) + assert.Equal(t, "com.example.TestService", info.InterfaceName) +} + +// Test GetServiceInfo returns nil for non-existent service +func TestGetServiceInfoNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + info := srv.GetServiceInfo("non-existent") + assert.Nil(t, info) +} + +// Test GetRPCService +func TestGetRPCService(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + mockService := &mockRPCService{} + svcOpts.rpcService = mockService + srv.registerServiceOptions(svcOpts) + + rpcService := srv.GetRPCService("test-service") + assert.NotNil(t, rpcService) + assert.Equal(t, mockService, rpcService) +} + +// Test GetRPCService returns nil for non-existent service +func TestGetRPCServiceNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + rpcService := srv.GetRPCService("non-existent") + assert.Nil(t, rpcService) +} + +// Test GetServiceOptionsByInterfaceName +func TestGetServiceOptionsByInterfaceName(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptionsByInterfaceName("com.example.TestService") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptionsByInterfaceName returns nil for non-existent interface +func TestGetServiceOptionsByInterfaceNameNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptionsByInterfaceName("non.existent.Service") + assert.Nil(t, retrieved) +} + +// Test registerServiceOptions +func TestRegisterServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + + srv.registerServiceOptions(svcOpts) + + // Verify via maps + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) Review Comment: Direct access to `srv.svcOptsMap` bypasses the mutex protection provided by the Server struct. This field is protected by `srv.mu` and should only be accessed through the public method `GetServiceOptions` or while holding the appropriate lock. While this test may not have concurrent access issues, accessing private fields directly in tests creates maintenance problems and violates encapsulation principles. ########## server/server_test.go: ########## @@ -0,0 +1,417 @@ +/* + * 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/common" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test NewServer creates a server successfully +func TestNewServer(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.NotNil(t, srv.cfg) + assert.NotNil(t, srv.svcOptsMap) + assert.NotNil(t, srv.interfaceNameServices) +} + +// Test NewServer with options +func TestNewServerWithOptions(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + } + srv, err := NewServer( + SetServerApplication(appCfg), + WithServerGroup("test-group"), + ) + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.Equal(t, appCfg, srv.cfg.Application) + assert.Equal(t, "test-group", srv.cfg.Provider.Group) +} + +// Test GetServiceOptions +func TestGetServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptions("test-service") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptions returns nil for non-existent service +func TestGetServiceOptionsNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptions("non-existent") + assert.Nil(t, retrieved) +} + +// Test GetServiceInfo +func TestGetServiceInfo(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.info = &common.ServiceInfo{ + InterfaceName: "com.example.TestService", + } + srv.registerServiceOptions(svcOpts) + + info := srv.GetServiceInfo("test-service") + assert.NotNil(t, info) + assert.Equal(t, "com.example.TestService", info.InterfaceName) +} + +// Test GetServiceInfo returns nil for non-existent service +func TestGetServiceInfoNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + info := srv.GetServiceInfo("non-existent") + assert.Nil(t, info) +} + +// Test GetRPCService +func TestGetRPCService(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + mockService := &mockRPCService{} + svcOpts.rpcService = mockService + srv.registerServiceOptions(svcOpts) + + rpcService := srv.GetRPCService("test-service") + assert.NotNil(t, rpcService) + assert.Equal(t, mockService, rpcService) +} + +// Test GetRPCService returns nil for non-existent service +func TestGetRPCServiceNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + rpcService := srv.GetRPCService("non-existent") + assert.Nil(t, rpcService) +} + +// Test GetServiceOptionsByInterfaceName +func TestGetServiceOptionsByInterfaceName(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptionsByInterfaceName("com.example.TestService") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptionsByInterfaceName returns nil for non-existent interface +func TestGetServiceOptionsByInterfaceNameNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptionsByInterfaceName("non.existent.Service") + assert.Nil(t, retrieved) +} + +// Test registerServiceOptions +func TestRegisterServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + + srv.registerServiceOptions(svcOpts) + + // Verify via maps + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Equal(t, svcOpts, srv.interfaceNameServices["com.example.TestService"]) +} + +// Test registerServiceOptions with empty interface +func TestRegisterServiceOptionsEmptyInterface(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "" + + srv.registerServiceOptions(svcOpts) + + // Should be in svcOptsMap but not in interfaceNameServices + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Nil(t, srv.interfaceNameServices[""]) Review Comment: Direct access to `srv.svcOptsMap` and `srv.interfaceNameServices` bypasses the mutex protection provided by the Server struct. These fields are protected by `srv.mu` and should only be accessed through the public methods like `GetServiceOptions` and `GetServiceOptionsByInterfaceName`, or while holding the appropriate lock. While this test may not have concurrent access issues, accessing private fields directly in tests creates maintenance problems and violates encapsulation principles. ########## server/server_test.go: ########## @@ -0,0 +1,417 @@ +/* + * 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/common" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test NewServer creates a server successfully +func TestNewServer(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.NotNil(t, srv.cfg) + assert.NotNil(t, srv.svcOptsMap) + assert.NotNil(t, srv.interfaceNameServices) +} + +// Test NewServer with options +func TestNewServerWithOptions(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + } + srv, err := NewServer( + SetServerApplication(appCfg), + WithServerGroup("test-group"), + ) + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.Equal(t, appCfg, srv.cfg.Application) + assert.Equal(t, "test-group", srv.cfg.Provider.Group) +} + +// Test GetServiceOptions +func TestGetServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptions("test-service") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptions returns nil for non-existent service +func TestGetServiceOptionsNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptions("non-existent") + assert.Nil(t, retrieved) +} + +// Test GetServiceInfo +func TestGetServiceInfo(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.info = &common.ServiceInfo{ + InterfaceName: "com.example.TestService", + } + srv.registerServiceOptions(svcOpts) + + info := srv.GetServiceInfo("test-service") + assert.NotNil(t, info) + assert.Equal(t, "com.example.TestService", info.InterfaceName) +} + +// Test GetServiceInfo returns nil for non-existent service +func TestGetServiceInfoNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + info := srv.GetServiceInfo("non-existent") + assert.Nil(t, info) +} + +// Test GetRPCService +func TestGetRPCService(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + mockService := &mockRPCService{} + svcOpts.rpcService = mockService + srv.registerServiceOptions(svcOpts) + + rpcService := srv.GetRPCService("test-service") + assert.NotNil(t, rpcService) + assert.Equal(t, mockService, rpcService) +} + +// Test GetRPCService returns nil for non-existent service +func TestGetRPCServiceNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + rpcService := srv.GetRPCService("non-existent") + assert.Nil(t, rpcService) +} + +// Test GetServiceOptionsByInterfaceName +func TestGetServiceOptionsByInterfaceName(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptionsByInterfaceName("com.example.TestService") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptionsByInterfaceName returns nil for non-existent interface +func TestGetServiceOptionsByInterfaceNameNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptionsByInterfaceName("non.existent.Service") + assert.Nil(t, retrieved) +} + +// Test registerServiceOptions +func TestRegisterServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + + srv.registerServiceOptions(svcOpts) + + // Verify via maps + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Equal(t, svcOpts, srv.interfaceNameServices["com.example.TestService"]) +} + +// Test registerServiceOptions with empty interface +func TestRegisterServiceOptionsEmptyInterface(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "" + + srv.registerServiceOptions(svcOpts) + + // Should be in svcOptsMap but not in interfaceNameServices + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Nil(t, srv.interfaceNameServices[""]) +} + +// Test SetProviderServices +func TestSetProviderServices(t *testing.T) { + // Reset the internal services slice + internalProServices = make([]*InternalService, 0, 16) + + internalService := &InternalService{ + Name: "test-internal-service", + Priority: 0, + Init: func(options *ServiceOptions) (*ServiceDefinition, bool) { + return &ServiceDefinition{ + Handler: &mockRPCService{}, + Info: nil, + Opts: []ServiceOption{}, + }, true + }, + } + + SetProviderServices(internalService) + + assert.Len(t, internalProServices, 1) + assert.Equal(t, "test-internal-service", internalProServices[0].Name) +} + +// Test SetProviderServices with empty name +func TestSetProviderServicesEmptyName(t *testing.T) { + // Reset the internal services slice + internalProServices = make([]*InternalService, 0, 16) Review Comment: This test modifies the global `internalProServices` variable but doesn't restore it after the test completes. If tests run in parallel or in a different order, this could cause test pollution and unpredictable failures. Consider using `t.Cleanup()` or `defer` to restore the original state, or better yet, isolate the test to avoid modifying global state. ########## server/server_test.go: ########## @@ -0,0 +1,417 @@ +/* + * 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/common" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test NewServer creates a server successfully +func TestNewServer(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.NotNil(t, srv.cfg) + assert.NotNil(t, srv.svcOptsMap) + assert.NotNil(t, srv.interfaceNameServices) +} + +// Test NewServer with options +func TestNewServerWithOptions(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + } + srv, err := NewServer( + SetServerApplication(appCfg), + WithServerGroup("test-group"), + ) + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.Equal(t, appCfg, srv.cfg.Application) + assert.Equal(t, "test-group", srv.cfg.Provider.Group) +} + +// Test GetServiceOptions +func TestGetServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptions("test-service") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptions returns nil for non-existent service +func TestGetServiceOptionsNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptions("non-existent") + assert.Nil(t, retrieved) +} + +// Test GetServiceInfo +func TestGetServiceInfo(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.info = &common.ServiceInfo{ + InterfaceName: "com.example.TestService", + } + srv.registerServiceOptions(svcOpts) + + info := srv.GetServiceInfo("test-service") + assert.NotNil(t, info) + assert.Equal(t, "com.example.TestService", info.InterfaceName) +} + +// Test GetServiceInfo returns nil for non-existent service +func TestGetServiceInfoNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + info := srv.GetServiceInfo("non-existent") + assert.Nil(t, info) +} + +// Test GetRPCService +func TestGetRPCService(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + mockService := &mockRPCService{} + svcOpts.rpcService = mockService + srv.registerServiceOptions(svcOpts) + + rpcService := srv.GetRPCService("test-service") + assert.NotNil(t, rpcService) + assert.Equal(t, mockService, rpcService) +} + +// Test GetRPCService returns nil for non-existent service +func TestGetRPCServiceNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + rpcService := srv.GetRPCService("non-existent") + assert.Nil(t, rpcService) +} + +// Test GetServiceOptionsByInterfaceName +func TestGetServiceOptionsByInterfaceName(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptionsByInterfaceName("com.example.TestService") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptionsByInterfaceName returns nil for non-existent interface +func TestGetServiceOptionsByInterfaceNameNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptionsByInterfaceName("non.existent.Service") + assert.Nil(t, retrieved) +} + +// Test registerServiceOptions +func TestRegisterServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + + srv.registerServiceOptions(svcOpts) + + // Verify via maps + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Equal(t, svcOpts, srv.interfaceNameServices["com.example.TestService"]) Review Comment: Direct access to `srv.interfaceNameServices` bypasses the mutex protection provided by the Server struct. This field is protected by `srv.mu` and should only be accessed through the public method `GetServiceOptionsByInterfaceName` or while holding the appropriate lock. While this test may not have concurrent access issues, accessing private fields directly in tests creates maintenance problems and violates encapsulation principles. ########## server/server_test.go: ########## @@ -0,0 +1,417 @@ +/* + * 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/common" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test NewServer creates a server successfully +func TestNewServer(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.NotNil(t, srv.cfg) + assert.NotNil(t, srv.svcOptsMap) + assert.NotNil(t, srv.interfaceNameServices) +} + +// Test NewServer with options +func TestNewServerWithOptions(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + } + srv, err := NewServer( + SetServerApplication(appCfg), + WithServerGroup("test-group"), + ) + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.Equal(t, appCfg, srv.cfg.Application) + assert.Equal(t, "test-group", srv.cfg.Provider.Group) +} + +// Test GetServiceOptions +func TestGetServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptions("test-service") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptions returns nil for non-existent service +func TestGetServiceOptionsNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptions("non-existent") + assert.Nil(t, retrieved) +} + +// Test GetServiceInfo +func TestGetServiceInfo(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.info = &common.ServiceInfo{ + InterfaceName: "com.example.TestService", + } + srv.registerServiceOptions(svcOpts) + + info := srv.GetServiceInfo("test-service") + assert.NotNil(t, info) + assert.Equal(t, "com.example.TestService", info.InterfaceName) +} + +// Test GetServiceInfo returns nil for non-existent service +func TestGetServiceInfoNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + info := srv.GetServiceInfo("non-existent") + assert.Nil(t, info) +} + +// Test GetRPCService +func TestGetRPCService(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + mockService := &mockRPCService{} + svcOpts.rpcService = mockService + srv.registerServiceOptions(svcOpts) + + rpcService := srv.GetRPCService("test-service") + assert.NotNil(t, rpcService) + assert.Equal(t, mockService, rpcService) +} + +// Test GetRPCService returns nil for non-existent service +func TestGetRPCServiceNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + rpcService := srv.GetRPCService("non-existent") + assert.Nil(t, rpcService) +} + +// Test GetServiceOptionsByInterfaceName +func TestGetServiceOptionsByInterfaceName(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptionsByInterfaceName("com.example.TestService") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptionsByInterfaceName returns nil for non-existent interface +func TestGetServiceOptionsByInterfaceNameNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptionsByInterfaceName("non.existent.Service") + assert.Nil(t, retrieved) +} + +// Test registerServiceOptions +func TestRegisterServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + + srv.registerServiceOptions(svcOpts) + + // Verify via maps + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Equal(t, svcOpts, srv.interfaceNameServices["com.example.TestService"]) +} + +// Test registerServiceOptions with empty interface +func TestRegisterServiceOptionsEmptyInterface(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "" + + srv.registerServiceOptions(svcOpts) + + // Should be in svcOptsMap but not in interfaceNameServices + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Nil(t, srv.interfaceNameServices[""]) +} + +// Test SetProviderServices +func TestSetProviderServices(t *testing.T) { Review Comment: This test modifies the global `internalProServices` variable but doesn't restore it after the test completes. If tests run in parallel or in a different order, this could cause test pollution and unpredictable failures. Consider using `t.Cleanup()` or `defer` to restore the original state, or better yet, isolate the test to avoid modifying global state. ```suggestion func TestSetProviderServices(t *testing.T) { // Preserve original internal services and restore after test origInternalProServices := internalProServices t.Cleanup(func() { internalProServices = origInternalProServices }) ``` ########## server/server_test.go: ########## @@ -0,0 +1,417 @@ +/* + * 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/common" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test NewServer creates a server successfully +func TestNewServer(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.NotNil(t, srv.cfg) + assert.NotNil(t, srv.svcOptsMap) + assert.NotNil(t, srv.interfaceNameServices) +} + +// Test NewServer with options +func TestNewServerWithOptions(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + } + srv, err := NewServer( + SetServerApplication(appCfg), + WithServerGroup("test-group"), + ) + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.Equal(t, appCfg, srv.cfg.Application) + assert.Equal(t, "test-group", srv.cfg.Provider.Group) +} + +// Test GetServiceOptions +func TestGetServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptions("test-service") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptions returns nil for non-existent service +func TestGetServiceOptionsNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptions("non-existent") + assert.Nil(t, retrieved) +} + +// Test GetServiceInfo +func TestGetServiceInfo(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.info = &common.ServiceInfo{ + InterfaceName: "com.example.TestService", + } + srv.registerServiceOptions(svcOpts) + + info := srv.GetServiceInfo("test-service") + assert.NotNil(t, info) + assert.Equal(t, "com.example.TestService", info.InterfaceName) +} + +// Test GetServiceInfo returns nil for non-existent service +func TestGetServiceInfoNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + info := srv.GetServiceInfo("non-existent") + assert.Nil(t, info) +} + +// Test GetRPCService +func TestGetRPCService(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + mockService := &mockRPCService{} + svcOpts.rpcService = mockService + srv.registerServiceOptions(svcOpts) + + rpcService := srv.GetRPCService("test-service") + assert.NotNil(t, rpcService) + assert.Equal(t, mockService, rpcService) +} + +// Test GetRPCService returns nil for non-existent service +func TestGetRPCServiceNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + rpcService := srv.GetRPCService("non-existent") + assert.Nil(t, rpcService) +} + +// Test GetServiceOptionsByInterfaceName +func TestGetServiceOptionsByInterfaceName(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptionsByInterfaceName("com.example.TestService") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptionsByInterfaceName returns nil for non-existent interface +func TestGetServiceOptionsByInterfaceNameNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptionsByInterfaceName("non.existent.Service") + assert.Nil(t, retrieved) +} + +// Test registerServiceOptions +func TestRegisterServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + + srv.registerServiceOptions(svcOpts) + + // Verify via maps + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Equal(t, svcOpts, srv.interfaceNameServices["com.example.TestService"]) +} + +// Test registerServiceOptions with empty interface +func TestRegisterServiceOptionsEmptyInterface(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "" + + srv.registerServiceOptions(svcOpts) + + // Should be in svcOptsMap but not in interfaceNameServices + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Nil(t, srv.interfaceNameServices[""]) +} + +// Test SetProviderServices +func TestSetProviderServices(t *testing.T) { + // Reset the internal services slice + internalProServices = make([]*InternalService, 0, 16) + + internalService := &InternalService{ + Name: "test-internal-service", + Priority: 0, + Init: func(options *ServiceOptions) (*ServiceDefinition, bool) { + return &ServiceDefinition{ + Handler: &mockRPCService{}, + Info: nil, + Opts: []ServiceOption{}, + }, true + }, + } + + SetProviderServices(internalService) + + assert.Len(t, internalProServices, 1) + assert.Equal(t, "test-internal-service", internalProServices[0].Name) +} + +// Test SetProviderServices with empty name +func TestSetProviderServicesEmptyName(t *testing.T) { + // Reset the internal services slice + internalProServices = make([]*InternalService, 0, 16) + + internalService := &InternalService{ + Name: "", + } + + SetProviderServices(internalService) + + assert.Len(t, internalProServices, 0) Review Comment: Direct access to the global `internalProServices` variable bypasses the mutex protection. The global variable `internalProServices` is protected by `internalProLock` in the production code (see server.go line 354-356). Reading it without holding the lock in tests creates a potential data race. While this test manipulates the variable before reading it, this pattern is unsafe and could lead to race conditions if tests run in parallel or if the implementation changes. ########## server/server_test.go: ########## @@ -0,0 +1,417 @@ +/* + * 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/common" + "dubbo.apache.org/dubbo-go/v3/global" +) + +// Test NewServer creates a server successfully +func TestNewServer(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.NotNil(t, srv.cfg) + assert.NotNil(t, srv.svcOptsMap) + assert.NotNil(t, srv.interfaceNameServices) +} + +// Test NewServer with options +func TestNewServerWithOptions(t *testing.T) { + appCfg := &global.ApplicationConfig{ + Name: "test-app", + } + srv, err := NewServer( + SetServerApplication(appCfg), + WithServerGroup("test-group"), + ) + assert.NoError(t, err) + assert.NotNil(t, srv) + assert.Equal(t, appCfg, srv.cfg.Application) + assert.Equal(t, "test-group", srv.cfg.Provider.Group) +} + +// Test GetServiceOptions +func TestGetServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptions("test-service") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptions returns nil for non-existent service +func TestGetServiceOptionsNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptions("non-existent") + assert.Nil(t, retrieved) +} + +// Test GetServiceInfo +func TestGetServiceInfo(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.info = &common.ServiceInfo{ + InterfaceName: "com.example.TestService", + } + srv.registerServiceOptions(svcOpts) + + info := srv.GetServiceInfo("test-service") + assert.NotNil(t, info) + assert.Equal(t, "com.example.TestService", info.InterfaceName) +} + +// Test GetServiceInfo returns nil for non-existent service +func TestGetServiceInfoNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + info := srv.GetServiceInfo("non-existent") + assert.Nil(t, info) +} + +// Test GetRPCService +func TestGetRPCService(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + mockService := &mockRPCService{} + svcOpts.rpcService = mockService + srv.registerServiceOptions(svcOpts) + + rpcService := srv.GetRPCService("test-service") + assert.NotNil(t, rpcService) + assert.Equal(t, mockService, rpcService) +} + +// Test GetRPCService returns nil for non-existent service +func TestGetRPCServiceNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + rpcService := srv.GetRPCService("non-existent") + assert.Nil(t, rpcService) +} + +// Test GetServiceOptionsByInterfaceName +func TestGetServiceOptionsByInterfaceName(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + srv.registerServiceOptions(svcOpts) + + retrieved := srv.GetServiceOptionsByInterfaceName("com.example.TestService") + assert.NotNil(t, retrieved) + assert.Equal(t, "test-service", retrieved.Id) +} + +// Test GetServiceOptionsByInterfaceName returns nil for non-existent interface +func TestGetServiceOptionsByInterfaceNameNotFound(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + retrieved := srv.GetServiceOptionsByInterfaceName("non.existent.Service") + assert.Nil(t, retrieved) +} + +// Test registerServiceOptions +func TestRegisterServiceOptions(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "com.example.TestService" + + srv.registerServiceOptions(svcOpts) + + // Verify via maps + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Equal(t, svcOpts, srv.interfaceNameServices["com.example.TestService"]) +} + +// Test registerServiceOptions with empty interface +func TestRegisterServiceOptionsEmptyInterface(t *testing.T) { + srv, err := NewServer() + assert.NoError(t, err) + + svcOpts := defaultServiceOptions() + svcOpts.Id = "test-service" + svcOpts.Service.Interface = "" + + srv.registerServiceOptions(svcOpts) + + // Should be in svcOptsMap but not in interfaceNameServices + assert.Equal(t, svcOpts, srv.svcOptsMap["test-service"]) + assert.Nil(t, srv.interfaceNameServices[""]) +} + +// Test SetProviderServices +func TestSetProviderServices(t *testing.T) { + // Reset the internal services slice + internalProServices = make([]*InternalService, 0, 16) + + internalService := &InternalService{ + Name: "test-internal-service", + Priority: 0, + Init: func(options *ServiceOptions) (*ServiceDefinition, bool) { + return &ServiceDefinition{ + Handler: &mockRPCService{}, + Info: nil, + Opts: []ServiceOption{}, + }, true + }, + } + + SetProviderServices(internalService) + + assert.Len(t, internalProServices, 1) + assert.Equal(t, "test-internal-service", internalProServices[0].Name) Review Comment: Direct access to the global `internalProServices` variable bypasses the mutex protection. The global variable `internalProServices` is protected by `internalProLock` in the production code (see server.go line 354-356). Reading it without holding the lock in tests creates a potential data race. While this test manipulates the variable before reading it, this pattern is unsafe and could lead to race conditions if tests run in parallel or if the implementation changes. -- 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]
