Fix infinite loop when consecutive releases return 304 Not Modified responses

.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

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)
 		}

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()
 

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
 }

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)
 		}
 	}

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)
+	}
+}

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)
 }

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