Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: 如果获取不到config，尝试远端同步获取一次，获取到的内容需要更新缓存 #301

Merged
merged 4 commits into from
Mar 7, 2024
Merged

fix: 如果获取不到config，尝试远端同步获取一次，获取到的内容需要更新缓存 #301

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
00c5a41
fix: 如果获取不到config，尝试远端同步获取一次，获取到的内容需要更新缓存
Mar 5, 2024
ac9ca83
fix: 优化 getConfigAndInit 逻辑，UpdateApolloConfig 的流程中已经包含了 cache store 过程
Mar 6, 2024
08f907f
fix: UT supplement
Mar 6, 2024
7022d70
fix: UT fix
Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func init() {

var syncApolloConfig = remote.CreateSyncApolloConfig()

//Client apollo 客户端接口
// Client apollo 客户端接口
type Client interface {
GetConfig(namespace string) *storage.Config
GetConfigAndInit(namespace string) *storage.Config
Expand Down Expand Up @@ -147,12 +147,12 @@ func StartWithConfig(loadAppConfig func() (*config.AppConfig, error)) (Client, e
return c, nil
}

//GetConfig 根据namespace获取apollo配置
// GetConfig 根据namespace获取apollo配置
func (c *internalClient) GetConfig(namespace string) *storage.Config {
return c.GetConfigAndInit(namespace)
}

//GetConfigAndInit 根据namespace获取apollo配置
// GetConfigAndInit 根据namespace获取apollo配置
func (c *internalClient) GetConfigAndInit(namespace string) *storage.Config {
if namespace == "" {
return nil
Expand All @@ -161,19 +161,19 @@ func (c *internalClient) GetConfigAndInit(namespace string) *storage.Config {
config := c.cache.GetConfig(namespace)

if config == nil {
//init cache
storage.CreateNamespaceConfig(namespace)

//sync config
syncApolloConfig.SyncWithNamespace(namespace, c.getAppConfig)
apolloConfig := syncApolloConfig.SyncWithNamespace(namespace, c.getAppConfig)
if apolloConfig != nil {
c.cache.UpdateApolloConfig(apolloConfig, c.getAppConfig)
}
}

config = c.cache.GetConfig(namespace)

return config
}

//GetConfigCache 根据namespace获取apollo配置的缓存
// GetConfigCache 根据namespace获取apollo配置的缓存
func (c *internalClient) GetConfigCache(namespace string) agcache.CacheInterface {
config := c.GetConfigAndInit(namespace)
if config == nil {
Expand All @@ -183,7 +183,7 @@ func (c *internalClient) GetConfigCache(namespace string) agcache.CacheInterface
return config.GetCache()
}

//GetDefaultConfigCache 获取默认缓存
// GetDefaultConfigCache 获取默认缓存
func (c *internalClient) GetDefaultConfigCache() agcache.CacheInterface {
config := c.GetConfigAndInit(storage.GetDefaultNamespace())
if config != nil {
Expand All @@ -192,42 +192,42 @@ func (c *internalClient) GetDefaultConfigCache() agcache.CacheInterface {
return nil
}

//GetApolloConfigCache 获取默认namespace的apollo配置
// GetApolloConfigCache 获取默认namespace的apollo配置
func (c *internalClient) GetApolloConfigCache() agcache.CacheInterface {
return c.GetDefaultConfigCache()
}

//GetValue 获取配置
// GetValue 获取配置
func (c *internalClient) GetValue(key string) string {
return c.GetConfig(storage.GetDefaultNamespace()).GetValue(key)
}

//GetStringValue 获取string配置值
// GetStringValue 获取string配置值
func (c *internalClient) GetStringValue(key string, defaultValue string) string {
return c.GetConfig(storage.GetDefaultNamespace()).GetStringValue(key, defaultValue)
}

//GetIntValue 获取int配置值
// GetIntValue 获取int配置值
func (c *internalClient) GetIntValue(key string, defaultValue int) int {
return c.GetConfig(storage.GetDefaultNamespace()).GetIntValue(key, defaultValue)
}

//GetFloatValue 获取float配置值
// GetFloatValue 获取float配置值
func (c *internalClient) GetFloatValue(key string, defaultValue float64) float64 {
return c.GetConfig(storage.GetDefaultNamespace()).GetFloatValue(key, defaultValue)
}

//GetBoolValue 获取bool 配置值
// GetBoolValue 获取bool 配置值
func (c *internalClient) GetBoolValue(key string, defaultValue bool) bool {
return c.GetConfig(storage.GetDefaultNamespace()).GetBoolValue(key, defaultValue)
}

//GetStringSliceValue 获取[]string 配置值
// GetStringSliceValue 获取[]string 配置值
func (c *internalClient) GetStringSliceValue(key string, defaultValue []string) []string {
return c.GetConfig(storage.GetDefaultNamespace()).GetStringSliceValue(key, separator, defaultValue)
}

//GetIntSliceValue 获取[]int 配置值
// GetIntSliceValue 获取[]int 配置值
func (c *internalClient) GetIntSliceValue(key string, defaultValue []int) []int {
return c.GetConfig(storage.GetDefaultNamespace()).GetIntSliceValue(key, separator, defaultValue)
}
Expand Down
38 changes: 37 additions & 1 deletion client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,14 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"reflect"
"testing"
"time"

"github.com/agiledragon/gomonkey/v2"

"github.com/apolloconfig/agollo/v4/agcache/memory"
"github.com/apolloconfig/agollo/v4/component/remote"
"github.com/apolloconfig/agollo/v4/env/config"
"github.com/apolloconfig/agollo/v4/env/server"

Expand All @@ -37,7 +41,7 @@ import (

const testDefaultNamespace = "application"

//init param
// init param
func init() {
extension.SetCacheFactory(&memory.DefaultCacheFactory{})
}
Expand Down Expand Up @@ -352,3 +356,35 @@ func TestUseEventDispatch(t *testing.T) {
l := cache.GetChangeListeners()
Assert(t, l.Len(), Equal(1))
}

func TestGetConfigAndInitValNotNil(t *testing.T) {
var apc *remote.AbsApolloConfig
patch := gomonkey.ApplyMethod(reflect.TypeOf(apc), "SyncWithNamespace", func(_ *remote.AbsApolloConfig, namespace string, appConfigFunc func() config.AppConfig) *config.ApolloConfig {
return &config.ApolloConfig{
ApolloConnConfig: config.ApolloConnConfig{
AppID: "testID",
NamespaceName: "testNotFound",
},
Configurations: map[string]interface{}{"testKey": "testValue"},
}
})
defer patch.Reset()

client := createMockApolloConfig(120)
cf := client.GetConfig("testNotFound")
Assert(t, cf, NotNilVal())
// cache should be updated
Assert(t, client.cache.GetConfig("testNotFound"), NotNilVal())
}
Copy link

Choose a reason for hiding this comment

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

The test TestGetConfigAndInitValNotNil uses the gomonkey package to mock the SyncWithNamespace method of AbsApolloConfig. This test aims to verify that when a valid configuration is returned by the mocked method, the cache is updated accordingly. This directly addresses the PR's objective to ensure the cache is updated with remotely fetched content when local retrieval fails.

However, it's crucial to ensure that the test accurately reflects the conditions under which the cache should be updated. The test should also verify that the cache contains the expected configuration values, not just that it is non-nil.

Consider enhancing the test to assert that the cache contains the expected configuration values after the SyncWithNamespace method is called. This will provide a more thorough validation of the cache update mechanism.

+ Assert(t, client.cache.GetConfig("testNotFound").Configurations["testKey"], Equal("testValue"))
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
func TestGetConfigAndInitValNotNil(t *testing.T) {
var apc *remote.AbsApolloConfig
patch := gomonkey.ApplyMethod(reflect.TypeOf(apc), "SyncWithNamespace", func(_ *remote.AbsApolloConfig, namespace string, appConfigFunc func() config.AppConfig) *config.ApolloConfig {
return &config.ApolloConfig{
ApolloConnConfig: config.ApolloConnConfig{
AppID: "testID",
NamespaceName: "testNotFound",
},
Configurations: map[string]interface{}{"testKey": "testValue"},
}
})
defer patch.Reset()
client := createMockApolloConfig(120)
cf := client.GetConfig("testNotFound")
Assert(t, cf, NotNilVal())
// cache should be updated
Assert(t, client.cache.GetConfig("testNotFound"), NotNilVal())
}
func TestGetConfigAndInitValNotNil(t *testing.T) {
var apc *remote.AbsApolloConfig
patch := gomonkey.ApplyMethod(reflect.TypeOf(apc), "SyncWithNamespace", func(_ *remote.AbsApolloConfig, namespace string, appConfigFunc func() config.AppConfig) *config.ApolloConfig {
return &config.ApolloConfig{
ApolloConnConfig: config.ApolloConnConfig{
AppID: "testID",
NamespaceName: "testNotFound",
},
Configurations: map[string]interface{}{"testKey": "testValue"},
}
})
defer patch.Reset()
client := createMockApolloConfig(120)
cf := client.GetConfig("testNotFound")
Assert(t, cf, NotNilVal())
// cache should be updated
Assert(t, client.cache.GetConfig("testNotFound"), NotNilVal())
Assert(t, client.cache.GetConfig("testNotFound").Configurations["testKey"], Equal("testValue"))
}


func TestGetConfigAndInitValNil(t *testing.T) {
var apc *remote.AbsApolloConfig
patch := gomonkey.ApplyMethod(reflect.TypeOf(apc), "SyncWithNamespace", func(_ *remote.AbsApolloConfig, namespace string, appConfigFunc func() config.AppConfig) *config.ApolloConfig {
return nil
})
defer patch.Reset()

client := createMockApolloConfig(120)
cf := client.GetConfig("testNotFound")
Assert(t, cf, NilVal())
}
Copy link

Choose a reason for hiding this comment

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

The test TestGetConfigAndInitValNil similarly uses the gomonkey package to mock the SyncWithNamespace method of AbsApolloConfig, but in this case, it simulates a scenario where the method returns nil. This test aims to verify that the configuration fetched is nil when the remote source does not provide a valid configuration.

This test is important for ensuring that the system behaves predictably in scenarios where the remote configuration source is unavailable or returns an invalid response. However, it would be beneficial to also verify that the cache is not updated in this scenario, which is not currently covered by the test.

Add an assertion to verify that the cache is not updated when SyncWithNamespace returns nil. This will ensure that the system's behavior aligns with expectations in error scenarios.

+ Assert(t, client.cache.GetConfig("testNotFound"), NilVal())
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
func TestGetConfigAndInitValNil(t *testing.T) {
var apc *remote.AbsApolloConfig
patch := gomonkey.ApplyMethod(reflect.TypeOf(apc), "SyncWithNamespace", func(_ *remote.AbsApolloConfig, namespace string, appConfigFunc func() config.AppConfig) *config.ApolloConfig {
return nil
})
defer patch.Reset()
client := createMockApolloConfig(120)
cf := client.GetConfig("testNotFound")
Assert(t, cf, NilVal())
}
func TestGetConfigAndInitValNil(t *testing.T) {
var apc *remote.AbsApolloConfig
patch := gomonkey.ApplyMethod(reflect.TypeOf(apc), "SyncWithNamespace", func(_ *remote.AbsApolloConfig, namespace string, appConfigFunc func() config.AppConfig) *config.ApolloConfig {
return nil
})
defer patch.Reset()
client := createMockApolloConfig(120)
cf := client.GetConfig("testNotFound")
Assert(t, cf, NilVal())
Assert(t, client.cache.GetConfig("testNotFound"), NilVal())
}

1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module github.com/apolloconfig/agollo/v4

require (
github.com/agiledragon/gomonkey/v2 v2.11.0 // indirect
github.com/spf13/viper v1.8.1
github.com/tevid/gohamcrest v1.1.1
)
Expand Down
Loading