fix: backup file has no content

Khachi-at commented 3 months ago

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
- Fix the issue of backup file has no content.
Tests
- Added tests for the new backup writing functionality to ensure reliable performance and error handling.

coderabbitai[bot] commented 3 months ago

Walkthrough

The updates introduce a WriteWithBackup method to enhance the ConfigFile functionalities, enabling configuration write operations with backup support. Corresponding tests have been added to ensure robustness. The changes affect methods and comments across several files in the config and file handling modules, ensuring uniformity and improved functionality.

Changes

File Path	Change Summary
`env/config/.../json_config.go`	Renamed comments, updated `Load` and `Write` methods, added `WriteWithBackup`.
`env/config/.../json_config_test.go`	Added tests `TestJSONConfigFile_WriteWithBackup` and `TestJSONConfigFile_WriteWithBackup_error`.
`env/file/.../json.go`, `.../raw.go`	Updated imports, comments, and replaced `Write` with `WriteWithBackup` in the `WriteConfigFile` method.

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

In the code, new winds do blow,
With backups in hand, our data flows,
Configs are safe, oh what a feat,
Tests are run, no risks to meet.
Embrace the change, with joy so sweet,
In lines of logic, the worlds do greet. 🌐✨

[!TIP]

AI model upgrade
## `gpt-4o` model for reviews and chat is now live OpenAI claims that this model is better at understanding and generating code than the previous models. Please join our [Discord Community](https://discord.com/invite/GsXnASn26c) to provide any feedback or to report any issues.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

Tips

### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

mergify[bot] commented 3 months ago

感谢您提出Pull Request，我会尽快Review。我会在1-2日内进行查看或者回复，如果遇到节假日可能会处理较慢，敬请谅解。

nobodyiam commented 3 months ago

Could you please help to describe the steps to duplicate the original bug?

coveralls commented 3 months ago

Pull Request Test Coverage Report for Build 9774866320

Details

9 of 9 (100.0%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.1%) to 75.709%

Totals
Change from base Build 8956224862:	0.1%
Covered Lines:	1334
Relevant Lines:	1762

💛 - Coveralls

Khachi-at commented 3 months ago

Could you please help to describe the steps to duplicate the original bug?

The test code is like this. You can duplicate the bug by running it.

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": "./"
}

nobodyiam commented 2 months ago

Could you please help to describe the steps to duplicate the original bug?

The test code is like this. You can duplicate the bug by running it.

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?

Khachi-at commented 2 months ago

Could you please help to describe the steps to duplicate the original bug?

The test code is like this. You can duplicate the bug by running it.

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.

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.
    agollo.StartWithConfig(func() (*config.AppConfig, error) {
        return appCfg, nil
    })
}

stale[bot] commented 1 week ago

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.

apolloconfig / agollo