Fix consistency bug where ClientConfig would get out of date between different parts of the code

This commit is contained in:
David Dworken
2023-09-23 12:40:57 -07:00
parent 8c239a32b9
commit ab12fa4d47
6 changed files with 40 additions and 14 deletions

View File

@@ -2290,7 +2290,7 @@ func TestSetConfigNoCorruption(t *testing.T) {
c.DeviceId = strings.Repeat("B", i*2) c.DeviceId = strings.Repeat("B", i*2)
c.HaveMissedUploads = (i % 2) == 0 c.HaveMissedUploads = (i % 2) == 0
// Write it // Write it
err := hctx.SetConfig(c) err := hctx.SetConfig(&c)
require.NoError(t, err) require.NoError(t, err)
// Check that we can read // Check that we can read
c2, err := hctx.GetConfig() c2, err := hctx.GetConfig()
@@ -2304,6 +2304,33 @@ func TestSetConfigNoCorruption(t *testing.T) {
doneWg.Wait() doneWg.Wait()
} }
// Test that the config retrieved from the context is a reference and there are no consistency issues with it getting out of sync
func TestCtxConfigIsReference(t *testing.T) {
// Setup
tester := zshTester{}
defer testutils.BackupAndRestore(t)()
installHishtory(t, tester, "")
// Get two copies of the conifg
ctx := hctx.MakeContext()
c1 := hctx.GetConf(ctx)
c2 := hctx.GetConf(ctx)
require.Equal(t, *c1, *c2)
// Change one and check that the other is changed
c1.LastSavedHistoryLine = "foobar"
require.Equal(t, c1.LastSavedHistoryLine, "foobar")
require.Equal(t, c2.LastSavedHistoryLine, "foobar")
// Persist that one, and then get the config again, and that one should also contain the change
require.NoError(t, hctx.SetConfig(c1))
c3 := hctx.GetConf(ctx)
require.Equal(t, *c1, *c3)
require.Equal(t, c1.LastSavedHistoryLine, "foobar")
require.Equal(t, c2.LastSavedHistoryLine, "foobar")
require.Equal(t, c3.LastSavedHistoryLine, "foobar")
}
func testMultipleUsers(t *testing.T, tester shellTester) { func testMultipleUsers(t *testing.T, tester shellTester) {
defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestore(t)()

View File

@@ -203,7 +203,7 @@ func handleUpgradedFeatures() error {
return err return err
} }
config.ControlRSearchEnabled = true config.ControlRSearchEnabled = true
return hctx.SetConfig(config) return hctx.SetConfig(&config)
} }
func installBinary(homedir string) (string, error) { func installBinary(homedir string) (string, error) {

View File

@@ -92,7 +92,7 @@ func handlePotentialUploadFailure(err error, config *hctx.ClientConfig, entryTim
// Set MissedUploadTimestamp to `entry timestamp - 1` so that the current entry will get // Set MissedUploadTimestamp to `entry timestamp - 1` so that the current entry will get
// uploaded once network access is regained. // uploaded once network access is regained.
config.MissedUploadTimestamp = entryTimestamp.UTC().Unix() - 1 config.MissedUploadTimestamp = entryTimestamp.UTC().Unix() - 1
lib.CheckFatalError(hctx.SetConfig(*config)) lib.CheckFatalError(hctx.SetConfig(config))
} }
} else { } else {
lib.CheckFatalError(err) lib.CheckFatalError(err)
@@ -143,7 +143,7 @@ func presaveHistoryEntry(ctx context.Context) {
jsonValue, err := lib.EncryptAndMarshal(config, []*data.HistoryEntry{entry}) jsonValue, err := lib.EncryptAndMarshal(config, []*data.HistoryEntry{entry})
lib.CheckFatalError(err) lib.CheckFatalError(err)
_, err = lib.ApiPost("/api/v1/submit?source_device_id="+config.DeviceId, "application/json", jsonValue) _, err = lib.ApiPost("/api/v1/submit?source_device_id="+config.DeviceId, "application/json", jsonValue)
handlePotentialUploadFailure(err, &config, entry.StartTime) handlePotentialUploadFailure(err, config, entry.StartTime)
} }
} }
@@ -175,7 +175,7 @@ func saveHistoryEntry(ctx context.Context) {
jsonValue, err := lib.EncryptAndMarshal(config, []*data.HistoryEntry{entry}) jsonValue, err := lib.EncryptAndMarshal(config, []*data.HistoryEntry{entry})
lib.CheckFatalError(err) lib.CheckFatalError(err)
w, err := lib.ApiPost("/api/v1/submit?source_device_id="+config.DeviceId, "application/json", jsonValue) w, err := lib.ApiPost("/api/v1/submit?source_device_id="+config.DeviceId, "application/json", jsonValue)
handlePotentialUploadFailure(err, &config, entry.StartTime) handlePotentialUploadFailure(err, config, entry.StartTime)
if err == nil { if err == nil {
submitResponse := shared.SubmitResponse{} submitResponse := shared.SubmitResponse{}
err := json.Unmarshal(w, &submitResponse) err := json.Unmarshal(w, &submitResponse)

View File

@@ -28,7 +28,7 @@ var statusCmd = &cobra.Command{
}, },
} }
func printOnlineStatus(config hctx.ClientConfig) { func printOnlineStatus(config *hctx.ClientConfig) {
if config.IsOffline { if config.IsOffline {
fmt.Println("Sync Mode: Disabled") fmt.Println("Sync Mode: Disabled")
} else { } else {

View File

@@ -121,7 +121,7 @@ func MakeContext() context.Context {
if err != nil { if err != nil {
panic(fmt.Errorf("failed to retrieve config: %w", err)) panic(fmt.Errorf("failed to retrieve config: %w", err))
} }
ctx = context.WithValue(ctx, configCtxKey, config) ctx = context.WithValue(ctx, configCtxKey, &config)
db, err := OpenLocalSqliteDb() db, err := OpenLocalSqliteDb()
if err != nil { if err != nil {
panic(fmt.Errorf("failed to open local DB: %w", err)) panic(fmt.Errorf("failed to open local DB: %w", err))
@@ -135,10 +135,10 @@ func MakeContext() context.Context {
return ctx return ctx
} }
func GetConf(ctx context.Context) ClientConfig { func GetConf(ctx context.Context) *ClientConfig {
v := ctx.Value(configCtxKey) v := ctx.Value(configCtxKey)
if v != nil { if v != nil {
return v.(ClientConfig) return (v.(*ClientConfig))
} }
panic(fmt.Errorf("failed to find config in ctx")) panic(fmt.Errorf("failed to find config in ctx"))
} }
@@ -235,8 +235,7 @@ func GetConfig() (ClientConfig, error) {
return config, nil return config, nil
} }
func SetConfig(config ClientConfig) error { func SetConfig(config *ClientConfig) error {
// TODO: Currently there is a consistency bug here where we update fields in the ClientConfig, and we write that to disk, but we never re-store it in hctx
serializedConfig, err := json.Marshal(config) serializedConfig, err := json.Marshal(config)
if err != nil { if err != nil {
return fmt.Errorf("failed to serialize config: %w", err) return fmt.Errorf("failed to serialize config: %w", err)
@@ -269,7 +268,7 @@ func InitConfig() error {
} }
_, err = os.Stat(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH)) _, err = os.Stat(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH))
if errors.Is(err, os.ErrNotExist) { if errors.Is(err, os.ErrNotExist) {
return SetConfig(ClientConfig{}) return SetConfig(&ClientConfig{})
} }
return err return err
} }

View File

@@ -63,7 +63,7 @@ func Setup(userSecret string, isOffline bool) error {
config.DeviceId = uuid.Must(uuid.NewRandom()).String() config.DeviceId = uuid.Must(uuid.NewRandom()).String()
config.ControlRSearchEnabled = true config.ControlRSearchEnabled = true
config.IsOffline = isOffline config.IsOffline = isOffline
err := hctx.SetConfig(config) err := hctx.SetConfig(&config)
if err != nil { if err != nil {
return fmt.Errorf("failed to persist config to disk: %w", err) return fmt.Errorf("failed to persist config to disk: %w", err)
} }
@@ -563,7 +563,7 @@ func ReliableDbCreate(db *gorm.DB, entry data.HistoryEntry) error {
}) })
} }
func EncryptAndMarshal(config hctx.ClientConfig, entries []*data.HistoryEntry) ([]byte, error) { func EncryptAndMarshal(config *hctx.ClientConfig, entries []*data.HistoryEntry) ([]byte, error) {
var encEntries []shared.EncHistoryEntry var encEntries []shared.EncHistoryEntry
for _, entry := range entries { for _, entry := range entries {
encEntry, err := data.EncryptHistoryEntry(config.UserSecret, *entry) encEntry, err := data.EncryptHistoryEntry(config.UserSecret, *entry)