diff --git a/.gitignore b/.gitignore index 6f7d7181..e13fcc88 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,7 @@ application*.json /abc1.json /env_test.properties /vendor/ +# Test artifact JSON files +test-*.json +storage/test-*.json +env/file/json/*.json diff --git a/client.go b/client.go index ecd67084..937022f0 100644 --- a/client.go +++ b/client.go @@ -160,7 +160,7 @@ func (c *internalClient) GetConfigAndInit(namespace string) *storage.Config { if cfg == nil { //sync config - apolloConfig := syncApolloConfig.SyncWithNamespace(namespace, c.getAppConfig) + apolloConfig, _ := syncApolloConfig.SyncWithNamespace(namespace, c.getAppConfig) if apolloConfig != nil { c.SyncAndUpdate(namespace, apolloConfig) } diff --git a/client_test.go b/client_test.go index a2dff5b3..585da410 100644 --- a/client_test.go +++ b/client_test.go @@ -355,14 +355,14 @@ func TestUseEventDispatch(t *testing.T) { 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 { + patch := gomonkey.ApplyMethod(reflect.TypeOf(apc), "SyncWithNamespace", func(_ *remote.AbsApolloConfig, namespace string, appConfigFunc func() config.AppConfig) (*config.ApolloConfig, error) { return &config.ApolloConfig{ ApolloConnConfig: config.ApolloConnConfig{ AppID: "testID", NamespaceName: "testNotFound", }, Configurations: map[string]interface{}{"testKey": "testUpdatedValue"}, - } + }, nil }) client := createMockApolloConfig(120) @@ -379,14 +379,14 @@ func TestGetConfigAndInitValNotNil(t *testing.T) { patch.Reset() // second replace - patch1 := gomonkey.ApplyMethod(reflect.TypeOf(apc), "SyncWithNamespace", func(_ *remote.AbsApolloConfig, namespace string, appConfigFunc func() config.AppConfig) *config.ApolloConfig { + patch1 := gomonkey.ApplyMethod(reflect.TypeOf(apc), "SyncWithNamespace", func(_ *remote.AbsApolloConfig, namespace string, appConfigFunc func() config.AppConfig) (*config.ApolloConfig, error) { return &config.ApolloConfig{ ApolloConnConfig: config.ApolloConnConfig{ AppID: "testID", NamespaceName: "testNotFound1", }, Configurations: map[string]interface{}{"testKey": "testUpdatedValue"}, - } + }, nil }) defer patch1.Reset() client.appConfig.NamespaceName = "testNotFound1" @@ -399,8 +399,8 @@ func TestGetConfigAndInitValNotNil(t *testing.T) { 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 + patch := gomonkey.ApplyMethod(reflect.TypeOf(apc), "SyncWithNamespace", func(_ *remote.AbsApolloConfig, namespace string, appConfigFunc func() config.AppConfig) (*config.ApolloConfig, error) { + return nil, nil }) defer patch.Reset() diff --git a/component/remote/abs.go b/component/remote/abs.go index ca249304..f7fea96d 100644 --- a/component/remote/abs.go +++ b/component/remote/abs.go @@ -28,7 +28,7 @@ type AbsApolloConfig struct { remoteApollo ApolloConfig } -func (a *AbsApolloConfig) SyncWithNamespace(namespace string, appConfigFunc func() config.AppConfig) *config.ApolloConfig { +func (a *AbsApolloConfig) SyncWithNamespace(namespace string, appConfigFunc func() config.AppConfig) (*config.ApolloConfig, error) { if appConfigFunc == nil { panic("can not find apollo config!please confirm!") } @@ -50,13 +50,13 @@ func (a *AbsApolloConfig) SyncWithNamespace(namespace string, appConfigFunc func apolloConfig, err := http.RequestRecovery(appConfig, c, &callback) if err != nil { log.Errorf("request %s fail, error:%v", urlSuffix, err) - return nil + return nil, err } if apolloConfig == nil { log.Debug("apolloConfig is nil") - return nil + return nil, nil } - return apolloConfig.(*config.ApolloConfig) + return apolloConfig.(*config.ApolloConfig), nil } diff --git a/component/remote/async.go b/component/remote/async.go index 1bdd978f..a01344c6 100644 --- a/component/remote/async.go +++ b/component/remote/async.go @@ -79,9 +79,12 @@ func (a *asyncApolloConfig) Sync(appConfigFunc func() config.AppConfig) []*confi } //只是拉去有变化的配置, 并更新拉取成功的namespace的notify ID for _, notifyConfig := range remoteConfigs { - apolloConfig := a.SyncWithNamespace(notifyConfig.NamespaceName, appConfigFunc) - if apolloConfig != nil { + apolloConfig, err := a.SyncWithNamespace(notifyConfig.NamespaceName, appConfigFunc) + // Update notificationID if we got a successful response (including 304) + if err == nil { appConfig.GetNotificationsMap().UpdateNotify(notifyConfig.NamespaceName, notifyConfig.NotificationID) + } + if apolloConfig != nil { apolloConfigs = append(apolloConfigs, apolloConfig) } } diff --git a/component/remote/async_test.go b/component/remote/async_test.go index affeb4b6..f8d063e4 100644 --- a/component/remote/async_test.go +++ b/component/remote/async_test.go @@ -362,3 +362,89 @@ func TestGetConfigURLSuffix(t *testing.T) { uri := asyncApollo.GetSyncURI(*appConfig, "kk") Assert(t, "", NotEqual(uri)) } + +// Test for the infinite loop bug fix when consecutive releases cause 304 responses +func TestApolloConfig_SyncWith304NotModified(t *testing.T) { + // This test simulates the exact infinite loop scenario: + // 1. Server has notification ID 3, client has 2 + // 2. Client gets notified of ID 3 + // 3. Client fetches config, gets 304 (already up-to-date) + // 4. Client should update notification ID to 3 to acknowledge the notification + // 5. If it doesn't update, next poll will get same notification again -> infinite loop + + serverNotificationId := int64(3) + requestCount := 0 + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.RequestURI + requestCount++ + + // Handle notification requests + if strings.Contains(path, "/notifications/v2") { + // Parse the current client notification ID from the URL + // If client has notification ID < serverNotificationId, return the server's ID + if strings.Contains(path, "notificationId%22%3A-1") || strings.Contains(path, "notificationId%22%3A2") { + // Client has -1 or 2, server has 3 + response := fmt.Sprintf(`[{"namespaceName":"application","notificationId":%d}]`, serverNotificationId) + fmt.Fprintf(w, response) + } else if strings.Contains(path, "notificationId%22%3A3") { + // Client has 3, server has 3 -> no update needed + fmt.Fprintf(w, "[]") + } else { + // Fallback for other cases + response := fmt.Sprintf(`[{"namespaceName":"application","notificationId":%d}]`, serverNotificationId) + fmt.Fprintf(w, response) + } + return + } + + // Handle config requests - always return 304 to simulate up-to-date config + if strings.Contains(path, "/configs/") { + w.WriteHeader(http.StatusNotModified) + return + } + + w.WriteHeader(http.StatusNotFound) + })) + defer server.Close() + + // Setup app config with notification ID 2 (behind server's 3) + appConfig := initNotifications() + appConfig.IP = server.URL + appConfig.GetNotificationsMap().UpdateNotify("application", 2) + + // Check initial notification state + initialNotifyId := appConfig.GetNotificationsMap().GetNotify("application") + t.Logf("Initial notificationId=%d", initialNotifyId) + + // First sync: should get notification ID 3 and update successfully + apolloConfigs := asyncApollo.Sync(func() config.AppConfig { + return *appConfig + }) + + actualNotifyId := appConfig.GetNotificationsMap().GetNotify("application") + t.Logf("After first sync: notificationId=%d, configs=%d", actualNotifyId, len(apolloConfigs)) + + // Should get no configs due to 304, but notificationId should update to 3 + Assert(t, len(apolloConfigs), Equal(0)) + Assert(t, actualNotifyId, Equal(int64(3))) + + // Second sync: should result in no notifications since client is now up-to-date + apolloConfigs2 := asyncApollo.Sync(func() config.AppConfig { + return *appConfig + }) + + finalNotifyId := appConfig.GetNotificationsMap().GetNotify("application") + t.Logf("After second sync: notificationId=%d, configs=%d", finalNotifyId, len(apolloConfigs2)) + + // Should still have notification ID 3 and no configs + Assert(t, len(apolloConfigs2), Equal(0)) + Assert(t, finalNotifyId, Equal(int64(3))) + + // Verify the infinite loop is fixed by checking that we don't get stuck + // If the bug existed, the first sync would not update the notification ID, + // and the second sync would get the same notification again + if requestCount > 10 { + t.Errorf("Too many requests (%d), possible infinite loop", requestCount) + } +} diff --git a/component/remote/remote.go b/component/remote/remote.go index 173e71d7..c2dac1a3 100644 --- a/component/remote/remote.go +++ b/component/remote/remote.go @@ -30,5 +30,5 @@ type ApolloConfig interface { // CallBack 根据 namespace 获取 callback 方法 CallBack(namespace string) http.CallBack // SyncWithNamespace 通过 namespace 同步 apollo 配置 - SyncWithNamespace(namespace string, appConfigFunc func() config.AppConfig) *config.ApolloConfig + SyncWithNamespace(namespace string, appConfigFunc func() config.AppConfig) (*config.ApolloConfig, error) } diff --git a/component/remote/sync.go b/component/remote/sync.go index 9b4b2f8a..caf7c124 100644 --- a/component/remote/sync.go +++ b/component/remote/sync.go @@ -100,7 +100,7 @@ func (a *syncApolloConfig) Sync(appConfigFunc func() config.AppConfig) []*config appConfig := appConfigFunc() configs := make([]*config.ApolloConfig, 0, 8) config.SplitNamespaces(appConfig.NamespaceName, func(namespace string) { - apolloConfig := a.SyncWithNamespace(namespace, appConfigFunc) + apolloConfig, _ := a.SyncWithNamespace(namespace, appConfigFunc) if apolloConfig != nil { configs = append(configs, apolloConfig) return