fix: backup file has no content #312
Conversation
WalkthroughThe updates introduce a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigFile
participant FileSystem
User->>ConfigFile: WriteWithBackup(data)
ConfigFile->>FileSystem: Backup existing file
FileSystem-->ConfigFile: Backup success/failure
ConfigFile->>FileSystem: Write new data to file
FileSystem-->ConfigFile: Write success/failure
ConfigFile-->>User: Operation success/failure
Poem
Tip AI model upgrade
|
|
感谢您提出Pull Request，我会尽快Review。我会在1-2日内进行查看或者回复，如果遇到节假日可能会处理较慢，敬请谅解。 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- env/config/json/json_config.go (3 hunks)
- env/config/json/json_config_test.go (1 hunks)
- env/file/json/json.go (3 hunks)
- env/file/json/raw.go (4 hunks)
Additional context used
Gitleaks
env/config/json/json_config_test.go
175-175: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
185-185: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (8)
env/file/json/raw.go (1)
Line range hint
63-74: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
WriteConfigFileare updated accordingly.Verification successful
Function calls to
WriteConfigFileare correctly updated.
- The
WriteConfigFilefunction instorage/repository.gouses the updated handler implementation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `WriteConfigFile` match the new implementation. # Test: Search for the function usage. Expect: Only occurances of the new implementation. rg --type go $'WriteConfigFile'Length of output: 1756
Script:
#!/bin/bash # Extract and review the context of each `WriteConfigFile` call in the main codebase, excluding test files. # List all files with `WriteConfigFile` occurrences, excluding test files rg --type go --files-with-matches 'WriteConfigFile' | grep -v '_test.go' | xargs -I {} sh -c 'echo "File: {}"; rg --context 5 "WriteConfigFile" {}'Length of output: 1853
Script:
#!/bin/bash # Verify the implementation of `extension.GetFileHandler()` to ensure it returns an instance of the updated handler. # Search for the implementation of `GetFileHandler` in the `extension` package rg --type go 'func GetFileHandler' extension/Length of output: 106
Script:
#!/bin/bash # Extract and review the implementation of `GetFileHandler` in `extension/file.go`. # Display the implementation of `GetFileHandler` rg --context 10 'func GetFileHandler' extension/file.goLength of output: 331
Script:
#!/bin/bash # Search for calls to `SetFileHandler` and review the context to confirm the correct handler is being used. # Search for calls to `SetFileHandler` rg --context 5 'SetFileHandler' --type goLength of output: 8046
env/config/json/json_config.go (3)
Line range hint
50-63: LGTM!The
Writefunction correctly handles errors and logs them.
66-74: LGTM!The
WriteWithBackupfunction correctly implements the backup mechanism.
Line range hint
34-49: LGTM!The
Loadfunction correctly handles errors and logs them.env/file/json/json.go (2)
Line range hint
99-115: LGTM!The
LoadConfigFilefunction correctly handles errors and logs them.
78-79: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
WriteConfigFileare updated accordingly.Verification successful
LGTM! But verify the function usage in the codebase.
The code changes are approved.
However, ensure that all function calls to
WriteConfigFileare updated accordingly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `WriteConfigFile` match the new implementation. # Test: Search for the function usage. Expect: Only occurances of the new implementation. rg --type go $'WriteConfigFile'Length of output: 1756
Script:
#!/bin/bash # Description: Verify the implementation of WriteConfigFile in the listed files. # Check implementation in start_test.go rg -A 5 -B 5 'func (fileHandler \*testFileHandler) WriteConfigFile' start_test.go # Check implementation in storage/repository.go rg -A 5 -B 5 'WriteConfigFile' storage/repository.go # Check implementation in env/file/file_handler.go rg -A 5 -B 5 'WriteConfigFile' env/file/file_handler.go # Check implementation in env/file/json/json.go rg -A 5 -B 5 'func (fileHandler \*FileHandler) WriteConfigFile' env/file/json/json.go # Check implementation in env/file/json/raw.go rg -A 5 -B 5 'func (fileHandler \*rawFileHandler) WriteConfigFile' env/file/json/raw.go # Check implementation in env/file/json/raw_test.go rg -A 5 -B 5 'WriteConfigFile' env/file/json/raw_test.go # Check implementation in env/file/json/json_test.go rg -A 5 -B 5 'WriteConfigFile' env/file/json/json_test.go # Check implementation in extension/file_test.go rg -A 5 -B 5 'func (r \*TestFileHandler) WriteConfigFile' extension/file_test.goLength of output: 3594
Script:
#!/bin/bash # Description: Verify the implementation of WriteConfigFile in the specific file env/file/json/json.go. # Check implementation in env/file/json/json.go rg -A 10 -B 10 'func (fileHandler \*FileHandler) WriteConfigFile' env/file/json/json.go # Check other instances to ensure they align with the new logic. rg -A 10 -B 10 'WriteConfigFile' env/file/json/raw.go env/file/json/raw_test.go env/file/json/json_test.go storage/repository.goLength of output: 8728
Script:
#!/bin/bash # Description: Verify the implementation of WriteConfigFile in the specific file env/file/json/json.go. # Check implementation in env/file/json/json.go rg -A 10 -B 10 'WriteWithBackup' env/file/json/json.goLength of output: 766
env/config/json/json_config_test.go (2)
173-181: LGTM!The
TestJSONConfigFile_WriteWithBackupfunction correctly tests theWriteWithBackupmethod.Tools
Gitleaks
175-175: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
183-194: LGTM!The
TestJSONConfigFile_WriteWithBackup_errorfunction correctly tests theWriteWithBackupmethod for error scenarios.Tools
Gitleaks
185-185: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
|
Could you please help to describe the steps to duplicate the original bug? |
Pull Request Test Coverage Report for Build 9774866320Details
💛 - Coveralls |
package apollofx
import (
"encoding/json"
"testing"
"github.com/apolloconfig/agollo/v4"
"github.com/apolloconfig/agollo/v4/env/config"
ajson "github.com/apolloconfig/agollo/v4/env/config/json"
"github.com/stretchr/testify/require"
)
func TestXxx(t *testing.T) {
loader := &ajson.ConfigFile{}
// Create app config.
appCfg := &config.AppConfig{}
_, err := loader.Load("config.json", func(bytes []byte) (interface{}, error) {
return appCfg, json.Unmarshal(bytes, appCfg)
})
require.NoError(t, err)
// Start apollo client.
client, err := agollo.StartWithConfig(func() (*config.AppConfig, error) {
return appCfg, nil
})
require.NoError(t, err)
_, err = client.GetConfigCache("namespace").Get("content")
require.NoError(t, err)
}config.json {
"appId": "testapp",
"cluster": "default",
"ip": "http://127.0.0.1:32026",
"namespaceName": "namespace",
"isBackupConfig": true,
"secret": "secret",
"backupConfigPath": "./"
} |
I followed the steps and found no errors. Did I miss something?
|
Adjusting the code like this can increase the probability of duplicating the bug. You need to pay attention to the contents of the backup file. Because a new backup file is created each time, if the program is stopped after the new backup file is created and before the content is written to the backup file, the backup file will have no content. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
It is found that the backup file has no content in use. The specific reason is that the file writing process stops (such as closing the process).
Based on the above reasons, the backup file writing logic is optimized, and the configuration file content is first written to a new file, and then the new file is used to overwrite the old file.
Summary by CodeRabbit
New Features
Tests