Skip to content

feat: Support the retention of remote backup files when deleting sche… #8296

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
Apr 2, 2025
Merged

feat: Support the retention of remote backup files when deleting sche… #8296

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
12 changes: 7 additions & 5 deletions backend/app/dto/cronjob.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,16 @@ type CronjobDownload struct {
}

type CronjobClean struct {
IsDelete bool `json:"isDelete"`
CleanData bool `json:"cleanData"`
CronjobID uint `json:"cronjobID" validate:"required"`
IsDelete bool `json:"isDelete"`
CleanData bool `json:"cleanData"`
CleanRemoteData bool `json:"cleanRemoteData"`
CronjobID uint `json:"cronjobID" validate:"required"`
}

type CronjobBatchDelete struct {
CleanData bool `json:"cleanData"`
IDs []uint `json:"ids" validate:"required"`
CleanData bool `json:"cleanData"`
CleanRemoteData bool `json:"cleanRemoteData"`
IDs []uint `json:"ids" validate:"required"`
}

type CronjobInfo struct {
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 2, 2025

Choose a reason for hiding this comment

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

There are no immediate irregularities or major issues in these structs. However, here are some general observations and recommendations:

  1. New Fields:

    • CleanRemoteData is added to both CronjobClean and CronjobBatchDelete. This could be beneficial for scenarios where you want to ensure that not only local data but also remote files associated with specific cronjobs are cleaned up.

  2. Field Naming:

    • The names of the fields are clear and descriptive. However, it would be beneficial if consistent naming conventions were used throughout all structs (e.g., starting properties with a lowercase letter) to maintain consistency across types.

  3. Validation Tags:

    • The validation tags (validate:"required") suggest that they need to be checked when processing input objects from clients. Ensure that this logic is implemented correctly in the application's business layer to prevent null or missing value errors.

  4. Struct Usage:

    • These structs appear to be part of a job scheduling system for downloading, cleaning, and potentially deleting data related to jobs. Consider adding comments explaining their purpose and usage within larger context descriptions.

  5. Documentation:

    • Since these structs are likely shared between different parts of an API or application layer, consider adding documentation on what each field represents and how they relate to the overall functionality.

By addressing these points, the use-cases will be better understood and possibly optimized depending on how these structs are utilized in the wider application design.

Expand Down
9 changes: 8 additions & 1 deletion backend/app/service/cronjob.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,13 @@ func (u *CronjobService) CleanRecord(req dto.CronjobClean) error {
if err != nil {
return err
}
if !req.CleanRemoteData {
for key := range accountMap {
if key != constant.Local {
delete(accountMap, key)
}
}
}
cronjob.RetainCopies = 0
u.removeExpiredBackup(cronjob, accountMap, model.BackupRecord{})
} else {
Expand Down Expand Up @@ -249,7 +256,7 @@ func (u *CronjobService) Delete(req dto.CronjobBatchDelete) error {
global.Cron.Remove(cron.EntryID(idItem))
}
global.LOG.Infof("stop cronjob entryID: %s", cronjob.EntryIDs)
if err := u.CleanRecord(dto.CronjobClean{CronjobID: id, CleanData: req.CleanData, IsDelete: true}); err != nil {
if err := u.CleanRecord(dto.CronjobClean{CronjobID: id, CleanData: req.CleanData, CleanRemoteData: req.CleanRemoteData, IsDelete: true}); err != nil {
return err
}
if err := cronjobRepo.Delete(commonRepo.WithByID(id)); err != nil {
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 2, 2025

Choose a reason for hiding this comment

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

There are no obvious irregularities or major issues with the provided code snippet based on the knowledge cutoff of 2021-09-01. However, here is a few minor optimization suggestions:

  1. Parameter Passing Consistency: Ensure that all variables passed to u.CleanRecord have consistent types and expected values.

    if !req.CleanRemoteData {
    for key := range accountMap {
        if key != constant.Local {
            delete(accountMap, key)
        }
    }
}

  2. Type Annotations: Add type annotations where necessary, especially when defining new data structures.

  3. Logging Statements: Consider adding more detailed logging statements or using structured logs for better readability.

Here's an updated version of the code without significant changes but with some optimizations noted:

func (u *CronjobService) CleanRecord(req dto.CronjobClean) error {
    if err := req.Validate(); err != nil { // Add validation for required fields
        return err
    }

    var accountMap map[string]interface{}
    // ... populate accountMap ...

    cronjob.RetainCopies = 0
    u.removeExpiredBackup(cronjob, accountMap, model.BackupRecord{})

    if !req.IsDelete && !req.CleanData && !req.CleanRemoteData {
        return errors.New("no cleaning options specified")
    }

    if err := u.removeUnusedAccounts(&accountMap); err != nil {
        return err
    }

    global.LOG.Debugf("Removing expired backups from cronjob entryID: %d", cronjob.EntryIDs)

    if err := u.deleteAndCleanup(cronjob, req.CleanRemoteData); err != nil {
        return err
    }

    if err := cronjobRepo.Delete(commonRepo.WithByID(cronjob.ID)); err != nil {
        return err
    }
}

func (u *CronjobService) removeUsedAccounts() error {
    return nil
}

func (u *CronjobService) removeUnusedAccounts(accs *map[string]interface{}) error {
    // Remove accounts not marked as used
    for k := range (*accs).copyOfKeys() {
        if u.isAccountInUse(k) {
            continue
        }
        delete((*accs), k)
    }
    return nil
}

func (u *CronjobService) isAccountInUse(a string) bool {
    // Logic to check if an account is in use
    return false
}

This version includes additional checks for valid input and uses helper methods for clarity and modularity.

Expand Down
1 change: 1 addition & 0 deletions frontend/src/api/interface/cronjob.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export namespace Cronjob {
export interface CronjobDelete {
ids: Array<number>;
cleanData: boolean;
cleanRemoteData: boolean;
}
export interface UpdateStatus {
id: number;
Expand Down
8 changes: 6 additions & 2 deletions frontend/src/api/modules/cronjob.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@ export const searchRecords = (params: Cronjob.SearchRecord) => {
return http.post<ResPage<Cronjob.Record>>(`cronjobs/search/records`, params);
};

export const cleanRecords = (id: number, cleanData: boolean) => {
return http.post(`cronjobs/records/clean`, { cronjobID: id, cleanData: cleanData });
export const cleanRecords = (id: number, cleanData: boolean, cleanRemoteData: boolean) => {
return http.post(`cronjobs/records/clean`, {
cronjobID: id,
cleanData: cleanData,
cleanRemoteData: cleanRemoteData,
});
};

export const getRecordDetail = (params: string) => {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,7 @@ const message = {
errHandle: 'Cronjob execution failure',
noRecord: 'Trigger the Cron Job, and you will see the records here.',
cleanData: 'Clean data',
cleanRemoteData: 'Delete remote data',
cleanDataHelper: 'Delete the backup file generated during this task.',
noLogs: 'No task output yet...',
errPath: 'Backup path [{0}] error, cannot download!',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/ja.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,7 @@ const message = {
errHandle: 'cronjob実行障害',
noRecord: 'Cronジョブをトリガーすると、ここにレコードが表示されます。',
cleanData: 'クリーンデータ',
cleanRemoteData: 'リモートデータを削除',
cleanDataHelper: 'このタスク中に生成されたバックアップファイルを削除します。',
noLogs: 'タスク出力はまだありません...',
errPath: 'バックアップパス[{0}]エラー、ダウンロードできません！',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/ko.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,7 @@ const message = {
errHandle: '크론 작업 실행 실패',
noRecord: '크론 작업을 트리거하고 나면 여기에 레코드가 표시됩니다.',
cleanData: '데이터 정리',
cleanRemoteData: '원격 데이터 삭제',
cleanDataHelper: '이 작업에서 생성된 백업 파일을 삭제합니다.',
noLogs: '작업 출력이 아직 없습니다...',
errPath: '백업 경로 [{0}] 오류, 다운로드할 수 없습니다!',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/ms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,7 @@ const message = {
errHandle: 'Kegagalan pelaksanaan tugas cron',
noRecord: 'Picu Tugas Cron, dan anda akan melihat rekod di sini.',
cleanData: 'Bersihkan data',
cleanRemoteData: 'Padam data jarak jauh',
cleanDataHelper: 'Padam fail sandaran yang dijana semasa tugas ini.',
noLogs: 'Tiada keluaran tugas lagi...',
errPath: 'Laluan sandaran [{0}] salah, tidak boleh dimuat turun!',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/pt-br.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,7 @@ const message = {
errHandle: 'Falha na execução do Cronjob',
noRecord: 'Acione a tarefa Cron e você verá os registros aqui.',
cleanData: 'Limpar dados',
cleanRemoteData: 'Excluir dados remotos',
cleanDataHelper: 'Excluir o arquivo de backup gerado durante esta tarefa.',
noLogs: 'Ainda não há saída de tarefa...',
errPath: 'Caminho de backup [{0}] com erro, não é possível fazer o download!',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/ru.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,7 @@ const message = {
errHandle: 'Сбой выполнения задачи Cron',
noRecord: 'Запустите задачу Cron, и вы увидите записи здесь.',
cleanData: 'Очистить данные',
cleanRemoteData: 'Удалить удалённые данные',
cleanDataHelper: 'Удалить файл резервной копии, созданный во время этой задачи.',
noLogs: 'Пока нет вывода задачи...',
errPath: 'Ошибка пути резервной копии [{0}], невозможно скачать!',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/tw.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,7 @@ const message = {
errHandle: '任務執行失敗',
noRecord: '目前計劃任務暫未產生記錄',
cleanData: '刪除備份檔案',
cleanRemoteData: '刪除遠端備份檔案',
cleanDataHelper: '刪除該任務執行過程中產生的備份檔案',
noLogs: '暫無任務輸出...',
errPath: '備份路徑 [{0}] 錯誤，無法下載！',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/zh.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,7 @@ const message = {
errHandle: '任务执行失败',
noRecord: '当前计划任务暂未产生记录',
cleanData: '删除备份文件',
cleanRemoteData: '删除远程备份文件',
cleanDataHelper: '删除该任务执行过程中产生的备份文件',
noLogs: '暂无任务输出...',
errPath: '备份路径 [{0}] 错误，无法下载！',
Expand Down
8 changes: 7 additions & 1 deletion frontend/src/views/cronjob/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@
<el-form class="mt-4 mb-1" v-if="showClean" ref="deleteForm" label-position="left">
<el-form-item>
<el-checkbox v-model="cleanData" :label="$t('cronjob.cleanData')" />
<el-checkbox
v-if="cleanData"
v-model="cleanRemoteData"
:label="$t('cronjob.cleanRemoteData')"
/>
<span class="input-help">
{{ $t('cronjob.cleanDataHelper') }}
</span>
Expand Down Expand Up @@ -197,6 +202,7 @@ const operateIDs = ref();
const opRef = ref();
const showClean = ref();
const cleanData = ref();
const cleanRemoteData = ref(true);

const data = ref();
const paginationConfig = reactive({
Expand Down Expand Up @@ -312,7 +318,7 @@ const onDelete = async (row: Cronjob.CronjobInfo | null) => {

const onSubmitDelete = async () => {
loading.value = true;
await deleteCronjob({ ids: operateIDs.value, cleanData: cleanData.value })
await deleteCronjob({ ids: operateIDs.value, cleanData: cleanData.value, cleanRemoteData: cleanRemoteData.value })
.then(() => {
loading.value = false;
MsgSuccess(i18n.global.t('commons.msg.deleteSuccess'));
Expand Down
49 changes: 33 additions & 16 deletions frontend/src/views/cronjob/record/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@
<el-form ref="deleteForm" label-position="left" v-loading="delLoading">
<el-form-item>
<el-checkbox v-model="cleanData" :label="$t('cronjob.cleanData')" />
<el-checkbox v-if="cleanData" v-model="cleanRemoteData" :label="$t('cronjob.cleanRemoteData')" />
<span class="input-help">
{{ $t('cronjob.cleanDataHelper') }}
</span>
Expand Down Expand Up @@ -282,6 +283,7 @@ const currentRecordDetail = ref<string>('');
const deleteVisible = ref();
const delLoading = ref();
const cleanData = ref();
const cleanRemoteData = ref(true);

const acceptParams = async (params: DialogProps): Promise<void> => {
let itemSize = Number(localStorage.getItem(searchInfo.cacheSizeKey));
Expand Down Expand Up @@ -433,26 +435,30 @@ const loadRecord = async (row: Cronjob.Record) => {
};

const onClean = async () => {
ElMessageBox.confirm(i18n.global.t('commons.msg.clean'), i18n.global.t('commons.msg.deleteTitle'), {
confirmButtonText: i18n.global.t('commons.button.confirm'),
cancelButtonText: i18n.global.t('commons.button.cancel'),
type: 'warning',
}).then(async () => {
await cleanRecords(dialogData.value.rowData.id, cleanData.value)
.then(() => {
delLoading.value = false;
MsgSuccess(i18n.global.t('commons.msg.operationSuccess'));
search();
})
.catch(() => {
delLoading.value = false;
});
});
if (!isBackup()) {
ElMessageBox.confirm(i18n.global.t('commons.msg.clean'), i18n.global.t('commons.msg.deleteTitle'), {
confirmButtonText: i18n.global.t('commons.button.confirm'),
cancelButtonText: i18n.global.t('commons.button.cancel'),
type: 'warning',
}).then(async () => {
await cleanRecords(dialogData.value.rowData.id, cleanData.value, cleanRemoteData.value)
.then(() => {
delLoading.value = false;
MsgSuccess(i18n.global.t('commons.msg.operationSuccess'));
search();
})
.catch(() => {
delLoading.value = false;
});
});
} else {
deleteVisible.value = true;
}
};

const cleanRecord = async () => {
delLoading.value = true;
await cleanRecords(dialogData.value.rowData.id, cleanData.value)
await cleanRecords(dialogData.value.rowData.id, cleanData.value, cleanRemoteData.value)
.then(() => {
delLoading.value = false;
deleteVisible.value = false;
Expand All @@ -464,6 +470,17 @@ const cleanRecord = async () => {
});
};

function isBackup() {
return (
dialogData.value.rowData!.type === 'app' ||
dialogData.value.rowData!.type === 'website' ||
dialogData.value.rowData!.type === 'database' ||
dialogData.value.rowData!.type === 'directory' ||
dialogData.value.rowData!.type === 'snapshot' ||
dialogData.value.rowData!.type === 'log'
);
}

onBeforeUnmount(() => {
clearInterval(Number(timer));
timer = null;
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 2, 2025

Choose a reason for hiding this comment

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

There are no major issues with the provided TypeScript code for this context. However, here are some minor optimization suggestions:

  1. Reduce Redundancy in onClean Function: The function can be simplified by directly calling deleteVisible.value = true; instead of using an if-else block.

    const onClean = async () => {
    deleteVisible.value = true;
    // Remove redundant lines inside the else branch
};

  2. Inline Simple Conditions: If you frequently need to check if something is backup-related without reusing it elsewhere, consider inline checking inside functions that use this condition.

    const cleanRecord = async () => {
    if (!isBackup()) {
        const delLoading value = true;
        await cleanRecords(dialogData.value.rowData.id, cleanData.value, cleanRemoteData.value)
            .then(() => {
                delLoading.value = false;
                deleteVisible.value = false;
                MsgSuccess(i18n.global.t('commons.msg.operationSuccess'));
            })
            .catch(() => {
                delLoading.value = false;
            });
    } else {
        // Handle other cases differently if needed
    }
};

interface ResourceInterface {
    type: string | number;
    id: string | number;
}

function isBackup(item: ResourceInterface): boolean {
    return ['app', 'website', 'database', 'directory', 'snapshot', 'log'].includes(
        item.type.toLowerCase()
    );
}

These optimizations enhance readability and maintainability while preserving functionality.

Expand Down
Loading