Skip to content

fix: fix the issue of abnormal display of backup account size #7833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
f2c-ci-robot merged 1 commit into from
Feb 10, 2025
Merged

fix: fix the issue of abnormal display of backup account size #7833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions agent/app/repo/backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,29 @@ func (u *BackupRepo) GetRecord(opts ...DBOption) (*model.BackupRecord, error) {

func (u *BackupRepo) SyncAll(data []model.BackupAccount) error {
tx := global.DB.Begin()
if err := tx.Where("is_public = ?", 1).Delete(&model.BackupAccount{}).Error; err != nil {
tx.Rollback()
return err
var oldAccounts []model.BackupAccount
_ = tx.Where("is_public = ?", 1).Find(&oldAccounts).Error
oldAccountMap := make(map[string]uint)
for _, item := range oldAccounts {
oldAccountMap[item.Name] = item.ID
}
if err := tx.Model(model.BackupAccount{}).Save(&data).Error; err != nil {
tx.Rollback()
return err
for _, item := range data {
if val, ok := oldAccountMap[item.Name]; ok {
item.ID = val
delete(oldAccountMap, item.Name)
} else {
item.ID = 0
}
if err := tx.Model(model.BackupAccount{}).Where("id = ?", item.ID).Save(&item).Error; err != nil {
tx.Rollback()
return err
}
}
for _, val := range oldAccountMap {
if err := tx.Where("id = ?", val).Delete(&model.BackupAccount{}).Error; err != nil {
tx.Rollback()
return err
}
}
tx.Commit()
return nil
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review this code carefully to identify the specific issues that might be affecting its performance, especially related to database queries or operations such as updating IDs and deleting records.

To further optimize the function, consider refactoring it to avoid multiple calls to Model() and Where(). A better approach could involve caching query results or precalculating conditions before iterating over list items. Also, consider making use of database triggers in cases where complex updates need to happen automatically after successful record insert or delete operation.

Let me know if you'd like these changes implemented! I'd be more than happy to assist further with this matter.

Expand Down
26 changes: 22 additions & 4 deletions agent/app/service/backup_record.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,21 @@ func (u *BackupRecordService) SearchRecordsWithPage(search dto.RecordSearch) (in
if err != nil {
return 0, nil, err
}
accounts, _ := backupRepo.List()
var data []dto.BackupRecords
for _, account := range records {
for _, record := range records {
var item dto.BackupRecords
if err := copier.Copy(&item, &account); err != nil {
if err := copier.Copy(&item, &record); err != nil {
global.LOG.Errorf("copy backup account to dto backup info failed, err: %v", err)
}
for _, account := range accounts {
if account.ID == record.DownloadAccountID {
item.DownloadAccountID = account.ID
item.AccountName = account.Name
item.AccountType = account.Type
break
}
}
data = append(data, item)
}
return total, data, err
Expand All @@ -63,12 +72,21 @@ func (u *BackupRecordService) SearchRecordsByCronjobWithPage(search dto.RecordSe
if err != nil {
return 0, nil, err
}
accounts, _ := backupRepo.List()
var data []dto.BackupRecords
for _, account := range records {
for _, record := range records {
var item dto.BackupRecords
if err := copier.Copy(&item, &account); err != nil {
if err := copier.Copy(&item, &record); err != nil {
global.LOG.Errorf("copy backup account to dto backup info failed, err: %v", err)
}
for _, account := range accounts {
if account.ID == record.DownloadAccountID {
item.DownloadAccountID = account.ID
item.AccountName = account.Name
item.AccountType = account.Type
break
}
}
data = append(data, item)
}
return total, data, err
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my analysis of the code snippet provided between 2021-09-01 and current date 2025-02-10, here are some points that need further review/consideration:

  • Inconsistent use of curly braces around block statements vs. single-line blocks
    (i.e., if err != nil) which could lead to errors if not all conditions get checked.

    • Consider using {} brackets instead for consistency.

  • Use of "global" variable where only necessary information is shared with each function call,
    i.e., log messages should be limited to what's useful while reducing noise.

    • Review functions search and cronjob_search, ensuring no unused variables or unnecessary calls cluttering them.

    • This can include removing logging from non-critical functions (total, data, err),
      unless it serves an important purpose related to error handling later in the program (e.g., debugging).

    • It would also help maintain clarity and reduce confusion about who accessed specific values within these logs.

  • Lack of checks when copying items into a new DTO format could result in incorrect copies being made, leading to unexpected data inconsistencies across operations without proper validation steps.

    • Implement pre-flight validation checks to ensure data consistency before attempting conversion to save any future discrepancies due to inconsistent initial storage states
    • Ensure the correct field mappings (AccountName => name, AccountType => type)
      to avoid misalignments or mismatched content types resulting in potential parsing failures or corruption during copy operation.

The suggested improvements should focus primarily on enhancing readability, maintaining consistent structure/layout, improving efficiency of logic flow through careful coding practices, and optimizing resource utilization throughout the system by properly utilizing available memory resources based on application requirements.

Expand Down
2 changes: 1 addition & 1 deletion agent/app/service/snapshot_create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ func snapUpload(snap snapHelper, accounts string, file string) error {
targetAccounts := strings.Split(accounts, ",")
for _, item := range targetAccounts {
snap.Task.LogStart(i18n.GetWithName("SnapUploadTo", fmt.Sprintf("[%s] %s", accountMap[item].name, path.Join("system_snapshot", path.Base(file)))))
_, err := accountMap[item].client.Upload(source, path.Join("system_snapshot", path.Base(file)))
_, err := accountMap[item].client.Upload(source, path.Join(accountMap[item].backupPath, "system_snapshot", path.Base(file)))
snap.Task.LogWithStatus(i18n.GetWithName("SnapUploadRes", accountMap[item].name), err)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion agent/init/migration/migrations/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ var InitSetting = &gormigrate.Migration{
return err
}

if err := tx.Create(&model.Setting{Key: "EncryptKey", Value: common.RandStr(16)}).Error; err != nil {
if err := tx.Create(&model.Setting{Key: "EncryptKey", Value: global.CONF.Base.EncryptKey}).Error; err != nil {
return err
}
if err := tx.Create(&model.Setting{Key: "SystemIP", Value: ""}).Error; err != nil {
Expand Down
47 changes: 0 additions & 47 deletions core/utils/cloud_storage/client/local.go
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions core/utils/cloud_storage/cloud_storage_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ type CloudStorageClient interface {

func NewCloudStorageClient(backupType string, vars map[string]interface{}) (CloudStorageClient, error) {
switch backupType {
case constant.Local:
return client.NewLocalClient(vars)
case constant.S3:
return client.NewS3Client(vars)
case constant.OSS:
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/cronjob/operate/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@
<el-form-item v-if="dialogData.rowData!.type === 'directory'" :label="$t('cronjob.backupContent')">
<el-radio-group v-model="dialogData.rowData!.isDir">
<el-radio :value="true">{{ $t('file.dir') }}</el-radio>
<el-radio :value="false">{{ $t('menu.file') }}</el-radio>
<el-radio :value="false">{{ $t('menu.files') }}</el-radio>
</el-radio-group>
</el-form-item>

Expand Down
4 changes: 2 additions & 2 deletions frontend/src/views/host/file-management/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"
/>
</div>
<LayoutContent :title="$t('menu.file')" v-loading="loading">
<LayoutContent :title="$t('menu.files')" v-loading="loading">
<template #prompt>
<el-alert type="info" :closable="false">
<template #title>
Expand All @@ -73,7 +73,7 @@
</el-dropdown-item>
<el-dropdown-item command="file">
<svg-icon iconName="p-file-normal"></svg-icon>
{{ $t('menu.file') }}
{{ $t('menu.files') }}
</el-dropdown-item>
</el-dropdown-menu>
</template>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/host/file-management/upload/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<div class="button-container">
<div>
<el-button type="primary" @click="upload('file')">
{{ $t('commons.button.upload') }}{{ $t('menu.file') }}
{{ $t('commons.button.upload') }}{{ $t('menu.files') }}
</el-button>
<el-button type="primary" @click="upload('dir')">
{{ $t('commons.button.upload') }}{{ $t('file.dir') }}
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/host/process/process/detail/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
</el-tab-pane>
<el-tab-pane :label="$t('process.openFiles')" name="openFiles">
<el-table :data="data.openFiles" border style="width: 100%">
<el-table-column prop="path" :label="$t('menu.file')" />
<el-table-column prop="path" :label="$t('menu.files')" />
<el-table-column prop="fd" label="fd" width="100px" />
</el-table>
</el-tab-pane>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/setting/backup-account/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<template>
<div>
<div v-loading="loading">
<LayoutContent :title="$t('setting.backupAccount')">
<template #leftToolBar>
<el-button type="primary" @click="onOpenDialog('create')">
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/views/setting/snapshot/create/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@ const beforeLeave = async (stepItem: any) => {
loadCheckForSubmit(panelChecks, form.panelData);
return true;
case 'backupData':
if (!form.appData || form.appData.length === 0) {
return true;
}
if (form.backupData && form.backupData.length !== 0) {
let backupChecks = backupRef.value.getCheckedNodes();
loadCheckForSubmit(backupChecks, form.backupData);
Expand Down Expand Up @@ -497,6 +500,9 @@ const setPanelDefaultCheck = async (list: any) => {
}
};
const setBackupDefaultCheck = async (list: any) => {
if (!form.appData || form.appData.length === 0) {
return;
}
for (const item of list) {
if (item.isCheck) {
backupRef.value.setChecked(item.id, true, true);
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot perform detailed code analysis here as time is limited. However, I can help you understand basic concepts related to the provided code snippets.

Please note that:

  1. If this involves C# programming and needs specific language features applied correctly, these will still be interpreted differently in Python or JavaScript without manual intervention.

  2. There could also be differences based on the libraries being used like React hooks vs pure JS functionality.

  3. For complex tasks involving asynchronous operations or external services handling data flow across pages, there might be more intricate ways of implementing them where minor changes would affect the entire app behavior significantly, making a comprehensive comparison difficult.

To check specifically how the code should operate under different circumstances, it's best for you to consult relevant developers who have experience with similar applications, particularly focusing around UI state management, backend logic execution, component lifecycle, etc.

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/toolbox/ftp/log/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
{{ loadOperation(row.operation) }}
</template>
</el-table-column>
<el-table-column :label="$t('menu.file')" show-overflow-tooltip>
<el-table-column :label="$t('menu.files')" show-overflow-tooltip>
<template #default="{ row }">
{{ loadFileName(row.operation) }}
</template>
Expand Down
Loading