From a62bb3181b75e71a9c1d11154a449ddd63eebc34 Mon Sep 17 00:00:00 2001 From: Jamie Sinn Date: Mon, 29 Apr 2024 09:55:48 -0400 Subject: [PATCH] fix: Send last modified header along with etag (#252) * Send last modified header along with etag * Add last modified to config set debug print * Set etag/lastmodified to be config properties specifically. --- bucketing/bucketing_test.go | 16 ++++++++-------- bucketing/config_manager.go | 12 ++++++++++-- bucketing/config_manager_test.go | 6 +++--- bucketing/event_queue_test.go | 20 ++++++++++---------- bucketing/model_config_body.go | 9 +++++---- bucketing/model_config_body_test.go | 2 +- client_native_bucketing.go | 8 ++++++-- client_test.go | 2 +- configmanager.go | 18 ++++++++++++++---- configmanager_test.go | 20 +++++++++++++++++++- testing_helpers.go | 2 ++ 11 files changed, 79 insertions(+), 36 deletions(-) diff --git a/bucketing/bucketing_test.go b/bucketing/bucketing_test.go index 7b2b66b0..5c7521c4 100644 --- a/bucketing/bucketing_test.go +++ b/bucketing/bucketing_test.go @@ -22,7 +22,7 @@ var ( // Bucketing puts the user in the target for the first audience they match func TestBucketingFirstMatchingTarget(t *testing.T) { - config, err := newConfig(test_config, "", "") + config, err := newConfig(test_config, "", "", "") require.NoError(t, err) feature := config.GetFeatureForVariableId("615356f120ed334a6054564c") @@ -53,7 +53,7 @@ func TestBucketing_RolloutGatesUser(t *testing.T) { Email: "test@email.com", }.GetPopulatedUser(&api.PlatformData{}) - config, err := newConfig(test_config, "", "") + config, err := newConfig(test_config, "", "", "") require.NoError(t, err) feature := config.GetFeatureForVariableId("61538237b0a70b58ae6af71f") @@ -190,7 +190,7 @@ func TestBucketing_Deterministic_RolloutNotEqualBucketing(t *testing.T) { func TestConfigParsing(t *testing.T) { // Parsing the large config should succeed without an error - err := SetConfig(test_config, "test", "etag", "rayid") + err := SetConfig(test_config, "test", "etag", "rayid", "lastModified") require.NoError(t, err) config, err := getConfig("test") require.NoError(t, err) @@ -468,7 +468,7 @@ func TestClientData(t *testing.T) { PlatformVersion: "1.1.2", }) - err := SetConfig(test_config, "test", "", "") + err := SetConfig(test_config, "test", "", "", "") require.NoError(t, err) // Ensure bucketed config has a feature variation map that's empty @@ -572,7 +572,7 @@ func TestVariableForUser(t *testing.T) { PlatformVersion: "1.1.2", }) - err := SetConfig(test_config, "test", "", "") + err := SetConfig(test_config, "test", "", "", "") require.NoError(t, err) variableType, value, featureId, variationId, err := generateBucketedVariableForUser("test", user, "json-var", nil) @@ -585,7 +585,7 @@ func TestVariableForUser(t *testing.T) { } func TestGenerateBucketedConfig_MissingDistribution(t *testing.T) { - err := SetConfig(test_broken_config, "broken_config", "", "") + err := SetConfig(test_broken_config, "broken_config", "", "", "") require.NoError(t, err) user := api.User{ @@ -599,7 +599,7 @@ func TestGenerateBucketedConfig_MissingDistribution(t *testing.T) { } func TestGenerateBucketedConfig_MissingVariations(t *testing.T) { - err := SetConfig(test_broken_config, "broken_config", "", "") + err := SetConfig(test_broken_config, "broken_config", "", "", "") require.NoError(t, err) user := api.User{ @@ -617,7 +617,7 @@ func TestGenerateBucketedConfig_MissingVariations(t *testing.T) { } func TestGenerateBucketedConfig_MissingVariables(t *testing.T) { - err := SetConfig(test_broken_config, "broken_config", "", "") + err := SetConfig(test_broken_config, "broken_config", "", "", "") require.NoError(t, err) user := api.User{ diff --git a/bucketing/config_manager.go b/bucketing/config_manager.go index 13645250..5096a87d 100644 --- a/bucketing/config_manager.go +++ b/bucketing/config_manager.go @@ -34,6 +34,14 @@ func GetRayId(sdkKey string) string { return config.rayId } +func GetLastModified(sdkKey string) string { + config, err := getConfig(sdkKey) + if err != nil { + return "" + } + return config.lastModified +} + func GetRawConfig(sdkKey string) []byte { configMutex.RLock() defer configMutex.RUnlock() @@ -43,10 +51,10 @@ func GetRawConfig(sdkKey string) []byte { return nil } -func SetConfig(rawJSON []byte, sdkKey, etag string, rayId string, eventQueue ...*EventQueue) error { +func SetConfig(rawJSON []byte, sdkKey, etag, rayId, lastModified string, eventQueue ...*EventQueue) error { configMutex.Lock() defer configMutex.Unlock() - config, err := newConfig(rawJSON, etag, rayId) + config, err := newConfig(rawJSON, etag, rayId, lastModified) if err != nil { return err } diff --git a/bucketing/config_manager_test.go b/bucketing/config_manager_test.go index 6c0a0437..359d5168 100644 --- a/bucketing/config_manager_test.go +++ b/bucketing/config_manager_test.go @@ -8,7 +8,7 @@ import ( ) func TestSetConfig(t *testing.T) { - err := SetConfig(test_config, "test", "test_etag", "rayId") + err := SetConfig(test_config, "test", "test_etag", "rayId", "lastModified") require.NoError(t, err) setConfig, err := getConfig("test") @@ -16,7 +16,7 @@ func TestSetConfig(t *testing.T) { baseConfig := configBody{} err = json.Unmarshal(test_config, &baseConfig) require.NoError(t, err) - baseConfig.compile("test_etag", "rayId") + baseConfig.compile("test_etag", "rayId", "lastModified") require.True(t, setConfig.Equals(baseConfig)) } @@ -28,7 +28,7 @@ func TestGetConfig_Unset(t *testing.T) { } func TestGetConfig_Set(t *testing.T) { - err := SetConfig(test_config, "test3", "test_etag", "") + err := SetConfig(test_config, "test3", "test_etag", "rayId", "lastModified") require.NoError(t, err) config, err := getConfig("test3") diff --git a/bucketing/event_queue_test.go b/bucketing/event_queue_test.go index 1d261d83..b53d60fc 100644 --- a/bucketing/event_queue_test.go +++ b/bucketing/event_queue_test.go @@ -18,7 +18,7 @@ func BenchmarkEventQueue_QueueEvent(b *testing.B) { CustomType: "testingtype", UserId: "testing", } - err := SetConfig(test_config, "dvc_server_token_hash", "", "") + err := SetConfig(test_config, "dvc_server_token_hash", "", "", "") require.NoError(b, err) eq, err := NewEventQueue("dvc_server_token_hash", &api.EventQueueOptions{MaxEventQueueSize: b.N + 10}, (&api.PlatformData{}).Default()) require.NoError(b, err) @@ -37,7 +37,7 @@ func BenchmarkEventQueue_QueueEvent_WithDrop(b *testing.B) { CustomType: "testingtype", UserId: "testing", } - err := SetConfig(test_config, "dvc_server_token_hash", "", "") + err := SetConfig(test_config, "dvc_server_token_hash", "", "", "") require.NoError(b, err) eq, err := NewEventQueue("dvc_server_token_hash", &api.EventQueueOptions{MaxEventQueueSize: b.N / 2}, (&api.PlatformData{}).Default()) require.NoError(b, err) @@ -52,7 +52,7 @@ func TestEventQueue_MergeAggEventQueueKeys(t *testing.T) { eq, err := NewEventQueue("dvc_server_token_hash", &api.EventQueueOptions{}, (&api.PlatformData{}).Default()) require.NoError(t, err) // Parsing the large config should succeed without an error - err = SetConfig(test_config, "test", "", "") + err = SetConfig(test_config, "test", "", "", "") require.NoError(t, err) config, err := getConfig("test") require.NoError(t, err) @@ -64,7 +64,7 @@ func TestEventQueue_FlushEvents(t *testing.T) { // Test flush events by ensuring that all events are flushed // and that the number of events flushed is equal to the number // of events reported. - err := SetConfig(test_config, "test", "", "") + err := SetConfig(test_config, "test", "", "", "") require.NoError(t, err) config, err := getConfig("test") require.NoError(t, err) @@ -86,7 +86,7 @@ func TestEventQueue_ProcessUserEvent(t *testing.T) { UserId: "testing", }, } - err := SetConfig(test_config, "dvc_server_token_hash", "", "") + err := SetConfig(test_config, "dvc_server_token_hash", "", "", "") require.NoError(t, err) eq, err := NewEventQueue("dvc_server_token_hash", &api.EventQueueOptions{}, (&api.PlatformData{}).Default()) require.NoError(t, err) @@ -101,7 +101,7 @@ func TestEventQueue_ProcessAggregateEvent(t *testing.T) { featureId: "featurekey", variationId: "somevariation", } - err := SetConfig(test_config, "dvc_server_token_hash", "", "") + err := SetConfig(test_config, "dvc_server_token_hash", "", "", "") require.NoError(t, err) eq, err := NewEventQueue("dvc_server_token_hash", &api.EventQueueOptions{}, (&api.PlatformData{}).Default()) require.NoError(t, err) @@ -116,7 +116,7 @@ func TestEventQueue_AddToUserQueue(t *testing.T) { CustomType: "testingtype", UserId: "testing", } - err := SetConfig(test_config, "dvc_server_token_hash", "", "") + err := SetConfig(test_config, "dvc_server_token_hash", "", "", "") require.NoError(t, err) eq, err := NewEventQueue("dvc_server_token_hash", &api.EventQueueOptions{}, (&api.PlatformData{}).Default()) require.NoError(t, err) @@ -125,7 +125,7 @@ func TestEventQueue_AddToUserQueue(t *testing.T) { } func TestEventQueue_AddToAggQueue(t *testing.T) { - err := SetConfig(test_config, "dvc_server_token_hash", "", "") + err := SetConfig(test_config, "dvc_server_token_hash", "", "", "") require.NoError(t, err) eq, err := NewEventQueue("dvc_server_token_hash", &api.EventQueueOptions{FlushEventsInterval: time.Hour}, (&api.PlatformData{}).Default()) require.NoError(t, err) @@ -142,7 +142,7 @@ func TestEventQueue_UserMaxQueueDrop(t *testing.T) { CustomType: "testingtype", UserId: "testing", } - err := SetConfig(test_config, "dvc_server_token_hash", "", "") + err := SetConfig(test_config, "dvc_server_token_hash", "", "", "") require.NoError(t, err) eq, err := NewEventQueue("dvc_server_token_hash", &api.EventQueueOptions{ DisableAutomaticEventLogging: true, @@ -172,7 +172,7 @@ func TestEventQueue_QueueAndFlush(t *testing.T) { CustomType: "testingtype", UserId: "testing", } - err := SetConfig(test_config, "dvc_server_token_hash", "", "") + err := SetConfig(test_config, "dvc_server_token_hash", "", "", "") require.NoError(t, err) eq, err := NewEventQueue("dvc_server_token_hash", &api.EventQueueOptions{ FlushEventsInterval: time.Hour, diff --git a/bucketing/model_config_body.go b/bucketing/model_config_body.go index 5dc9cb2d..280f9cf5 100644 --- a/bucketing/model_config_body.go +++ b/bucketing/model_config_body.go @@ -32,12 +32,13 @@ type configBody struct { Variables []*Variable `json:"variables" validate:"required,dive"` etag string rayId string + lastModified string variableIdMap map[string]*Variable variableKeyMap map[string]*Variable variableIdToFeatureMap map[string]*ConfigFeature } -func newConfig(configJSON []byte, etag string, rayId string) (*configBody, error) { +func newConfig(configJSON []byte, etag, rayId, lastModified string) (*configBody, error) { config := configBody{} if err := json.Unmarshal(configJSON, &config); err != nil { return nil, err @@ -48,7 +49,7 @@ func newConfig(configJSON []byte, etag string, rayId string) (*configBody, error if config.Audiences == nil { config.Audiences = make(map[string]NoIdAudience) } - config.compile(etag, rayId) + config.compile(etag, rayId, lastModified) return &config, nil } @@ -73,7 +74,7 @@ func (c *configBody) GetFeatureForVariableId(id string) *ConfigFeature { return nil } -func (c *configBody) compile(etag string, rayId string) { +func (c *configBody) compile(etag, rayId, lastModified string) { // Build mappings of IDs and keys to features and variables. variableIdToFeatureMap := make(map[string]*ConfigFeature) for _, feature := range c.Features { @@ -98,7 +99,7 @@ func (c *configBody) compile(etag string, rayId string) { c.variableKeyMap = variableKeyMap c.etag = etag c.rayId = rayId - + c.lastModified = lastModified // Sort the feature distributions by "_variation" attribute in descending alphabetical order for _, feature := range c.Features { for _, target := range feature.Configuration.Targets { diff --git a/bucketing/model_config_body_test.go b/bucketing/model_config_body_test.go index d3658159..254c9d27 100644 --- a/bucketing/model_config_body_test.go +++ b/bucketing/model_config_body_test.go @@ -131,7 +131,7 @@ func TestParseConfigBody(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - result, err := newConfig([]byte(tc.inputJSON), "", "") + result, err := newConfig([]byte(tc.inputJSON), "", "", "") if tc.expectError { require.Error(t, err, "Expected error, got nil") diff --git a/client_native_bucketing.go b/client_native_bucketing.go index 84b9c7a9..f16f8c0d 100644 --- a/client_native_bucketing.go +++ b/client_native_bucketing.go @@ -53,13 +53,13 @@ func NewNativeLocalBucketing(sdkKey string, platformData *api.PlatformData, opti }, err } -func (n *NativeLocalBucketing) StoreConfig(configJSON []byte, eTag string, rayId string) error { +func (n *NativeLocalBucketing) StoreConfig(configJSON []byte, eTag, rayId, lastModified string) error { oldETag := bucketing.GetEtag(n.sdkKey) _, err := n.eventQueue.FlushEventQueue(n.clientUUID, oldETag, n.GetRayId()) if err != nil { return fmt.Errorf("Error flushing events for %s: %w", oldETag, err) } - err = bucketing.SetConfig(configJSON, n.sdkKey, eTag, rayId, n.eventQueue) + err = bucketing.SetConfig(configJSON, n.sdkKey, eTag, rayId, lastModified, n.eventQueue) if err != nil { return fmt.Errorf("Error parsing config: %w", err) } @@ -86,6 +86,10 @@ func (n *NativeLocalBucketing) GetClientUUID() string { return n.clientUUID } +func (n *NativeLocalBucketing) GetLastModified() string { + return bucketing.GetLastModified(n.sdkKey) +} + func (n *NativeLocalBucketing) GenerateBucketedConfigForUser(user User) (ret *BucketedUserConfig, err error) { populatedUser := user.GetPopulatedUserWithTime(n.platformData, DEFAULT_USER_TIME) clientCustomData := bucketing.GetClientCustomData(n.sdkKey) diff --git a/client_test.go b/client_test.go index 39f6e6d6..0e3174c6 100644 --- a/client_test.go +++ b/client_test.go @@ -554,7 +554,7 @@ func BenchmarkClient_VariableParallel(b *testing.B) { } if benchmarkEnableConfigUpdates && configCounter.Add(1)%10000 == 0 { go func() { - err = client.configManager.setConfig([]byte(test_large_config), "", "") + err = client.configManager.setConfig([]byte(test_large_config), "", "", "") setConfigCount.Add(1) }() } diff --git a/configmanager.go b/configmanager.go index 59d2dfa4..c534bfa3 100644 --- a/configmanager.go +++ b/configmanager.go @@ -14,9 +14,10 @@ import ( const CONFIG_RETRIES = 1 type ConfigReceiver interface { - StoreConfig([]byte, string, string) error + StoreConfig([]byte, string, string, string) error GetRawConfig() []byte GetETag() string + GetLastModified() string HasConfig() bool } @@ -94,10 +95,14 @@ func (e *EnvironmentConfigManager) fetchConfig(numRetriesRemaining int) (err err } etag := e.localBucketing.GetETag() + lastModified := e.localBucketing.GetLastModified() if etag != "" { req.Header.Set("If-None-Match", etag) } + if lastModified != "" { + req.Header.Set("If-Modified-Since", lastModified) + } resp, err := e.httpClient.Do(req) if err != nil { if numRetriesRemaining > 0 { @@ -151,13 +156,14 @@ func (e *EnvironmentConfigManager) setConfigFromResponse(response *http.Response config, response.Header.Get("Etag"), response.Header.Get("Cf-Ray"), + response.Header.Get("Last-Modified"), ) if err != nil { return err } - util.Infof("Config set. ETag: %s\n", e.localBucketing.GetETag()) + util.Infof("Config set. ETag: %s Last-Modified: %s\n", e.localBucketing.GetETag(), e.localBucketing.GetLastModified()) if e.firstLoad { e.firstLoad = false @@ -166,8 +172,8 @@ func (e *EnvironmentConfigManager) setConfigFromResponse(response *http.Response return nil } -func (e *EnvironmentConfigManager) setConfig(config []byte, eTag string, rayId string) error { - err := e.localBucketing.StoreConfig(config, eTag, rayId) +func (e *EnvironmentConfigManager) setConfig(config []byte, eTag, rayId, lastModified string) error { + err := e.localBucketing.StoreConfig(config, eTag, rayId, lastModified) if err != nil { return err } @@ -193,6 +199,10 @@ func (e *EnvironmentConfigManager) GetETag() string { return e.localBucketing.GetETag() } +func (e *EnvironmentConfigManager) GetLastModified() string { + return e.localBucketing.GetLastModified() +} + func (e *EnvironmentConfigManager) Close() { e.stopPolling() } diff --git a/configmanager_test.go b/configmanager_test.go index af63dcb0..b6acd0c2 100644 --- a/configmanager_test.go +++ b/configmanager_test.go @@ -12,12 +12,14 @@ type recordingConfigReceiver struct { configureCount int etag string rayId string + lastModified string } -func (r *recordingConfigReceiver) StoreConfig(_ []byte, etag string, rayId string) error { +func (r *recordingConfigReceiver) StoreConfig(_ []byte, etag, rayId, lastModified string) error { r.configureCount++ r.etag = etag r.rayId = rayId + r.lastModified = lastModified return nil } @@ -37,6 +39,10 @@ func (r *recordingConfigReceiver) GetRawConfig() []byte { return nil } +func (r *recordingConfigReceiver) GetLastModified() string { + return r.lastModified +} + func TestEnvironmentConfigManager_fetchConfig_success(t *testing.T) { httpmock.Activate() defer httpmock.DeactivateAndReset() @@ -82,6 +88,12 @@ func TestEnvironmentConfigManager_fetchConfig_retries500(t *testing.T) { if !manager.HasConfig() { t.Fatal("cm.hasConfig != true") } + if manager.GetETag() != "TESTING" { + t.Fatal("cm.configEtag != TESTING") + } + if manager.GetLastModified() != "LAST-MODIFIED" { + t.Fatal("cm.lastModified != LAST-MODIFIED") + } } func TestEnvironmentConfigManager_fetchConfig_retries_errors(t *testing.T) { @@ -104,6 +116,12 @@ func TestEnvironmentConfigManager_fetchConfig_retries_errors(t *testing.T) { if !manager.HasConfig() { t.Fatal("cm.hasConfig != true") } + if manager.GetETag() != "TESTING" { + t.Fatal("cm.configEtag != TESTING") + } + if manager.GetLastModified() != "LAST-MODIFIED" { + t.Fatal("cm.lastModified != LAST-MODIFIED") + } } func TestEnvironmentConfigManager_fetchConfig_returns_errors(t *testing.T) { diff --git a/testing_helpers.go b/testing_helpers.go index 5e3ec88c..fc8a63e8 100644 --- a/testing_helpers.go +++ b/testing_helpers.go @@ -45,6 +45,7 @@ func httpBucketingAPIMock() { resp := httpmock.NewStringResponse(200, `{"value": true, "_id": "614ef6ea475129459160721a", "key": "test", "type": "Boolean"}`) resp.Header.Set("Etag", "TESTING") + resp.Header.Set("Last-Modified", "LAST-MODIFIED") return resp, nil }, ) @@ -63,6 +64,7 @@ func httpCustomConfigMock(sdkKey string, respcode int, config string) httpmock.R responder := func(req *http.Request) (*http.Response, error) { resp := httpmock.NewStringResponse(respcode, config) resp.Header.Set("Etag", "TESTING") + resp.Header.Set("Last-Modified", "LAST-MODIFIED") return resp, nil } httpmock.RegisterResponder("GET", "https://config-cdn.devcycle.com/config/v1/server/"+sdkKey+".json", responder)