Skip to content

feat(app): delete parent dir after uninstall app #8350

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
wanghe-fit2cloud merged 1 commit into from
Apr 8, 2025
Merged

feat(app): delete parent dir after uninstall app #8350

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
4 changes: 2 additions & 2 deletions agent/app/repo/task.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ func (t TaskRepo) CountExecutingTask() (int64, error) {
return count, err
}

func (u TaskRepo) Delete(opts ...DBOption) error {
func (t TaskRepo) Delete(opts ...DBOption) error {
db := global.TaskDB
for _, opt := range opts {
db = opt(db)
}
return db.Delete(&model.Task{}).Error
}

func (u TaskRepo) DeleteAll() error {
func (t TaskRepo) DeleteAll() error {
return global.TaskDB.Where("1 = 1").Delete(&model.Task{}).Error
}
6 changes: 6 additions & 0 deletions agent/app/service/app_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"os"
"os/exec"
"path"
"path/filepath"
"reflect"
"regexp"
"strconv"
Expand Down Expand Up @@ -428,6 +429,11 @@ func deleteAppInstall(deleteReq request.AppInstallDelete) error {
}
}
_ = op.DeleteDir(appDir)
parentDir := filepath.Dir(appDir)
entries, err := os.ReadDir(parentDir)
if err == nil && len(entries) == 0 {
_ = op.DeleteDir(parentDir)
}
tx.Commit()
return nil
}
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 8, 2025

Choose a reason for hiding this comment

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

The added code to handle empty parent directories is beneficial for maintaining directory structure consistency. Here’s a brief summary of the changes:

  1. import "path/filepath" is added at the top of the file to use functions from this package.

  2. Inside the deleteAppInstall function:

    • If an application installation entry exists, it iterates through each instance associated with the app ID.
    • For each instance, it deletes files and folders using _ = op.DeleteDir(...).

  3. After deleting the child directory (appDir) within its parent, additional logic is implemented for handling an empty parent directory conditionally:

    • The full path of the parent directory is obtained using filepath.Dir(appDir).
    • An attempt is made to read all entries (files/folders) in that parent directory via os.ReadDir(parentDir).
    • If the parent directory has no entries left (i.e., there are no remaining files/folders), it attempts to delete it again.

Optimization Suggestions:

  • Consistent Error Handling: Ensure consistent error handling across the codebase to maintain reliability and prevent silent failures due to missing imports or unexpected errors.

  • Avoid Duplicated Code: Review if these checks can be reused elsewhere in the codebase to avoid duplication, potentially improving scalability and maintenance efficiency.

  • Testing: Consider adding comprehensive tests to cover scenarios where parent directories might remain non-empty after deletions, which could indicate other underlying issues in the system.

By incorporating these optimizations, the overall functionality remains robust while also making the code more reusable and maintainable over time.

Expand Down
3 changes: 3 additions & 0 deletions agent/app/service/runtime.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,9 @@ func (r *RuntimeService) InstallPHPExtension(req request.PHPExtensionInstallReq)
if err != nil {
return err
}
if task.CheckResourceTaskIsExecuting(task.TaskInstall, task.TaskScopeRuntimeExtension, runtime.ID) {
return buserr.New("ErrInstallExtension")
}
installTask, err := task.NewTaskWithOps(req.Name, task.TaskInstall, task.TaskScopeRuntimeExtension, req.TaskID, runtime.ID)
if err != nil {
return err
Expand Down
10 changes: 10 additions & 0 deletions agent/app/task/task.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,16 @@ func CheckTaskIsExecuting(name string) error {
return nil
}

func CheckResourceTaskIsExecuting(operate, scope string, resourceID uint) bool {
taskRepo := repo.NewITaskRepo()
task, _ := taskRepo.GetFirst(
taskRepo.WithByStatus(constant.StatusExecuting),
taskRepo.WithResourceID(resourceID),
taskRepo.WithOperate(operate),
repo.WithByType(scope))
return task.ID != ""
}

func NewTask(name, operate, taskScope, taskID string, resourceID uint) (*Task, error) {
if taskID == "" {
taskID = uuid.New().String()
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 8, 2025

Choose a reason for hiding this comment

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

  1. The function CheckResourceTaskIsExecuting is declared but not used anywhere in the provided code.
  2. The parameter scope is defined twice with different types (string and repo.ResourceScope). This may cause confusion unless they are meant to be of the same type.

Overall, there isn't significant irregularity or potential issue detected in this update snippet from April 2023.

Expand Down
5 changes: 4 additions & 1 deletion agent/i18n/lang/zh.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,4 +416,7 @@ ErrAlert: "告警信息格式错误，请检查后重试！"
ErrAlertPush: "告警信息推送错误，请检查后重试！"
ErrAlertSave: "告警信息保存错误，请检查后重试！"
ErrAlertSync: "告警信息同步错误，请检查后重试！"
ErrAlertRemote: "告警信息远端错误，请检查后重试！"
ErrAlertRemote: "告警信息远端错误，请检查后重试！"

#task - runtime
ErrInstallExtension: "已有安装任务正在进行，请等待任务结束"
4 changes: 2 additions & 2 deletions frontend/src/lang/modules/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2707,8 +2707,8 @@ const message = {
env: 'Environment',
noenv: 'None',
net: 'Network Connection',
laddr: 'Source address/port',
raddr: 'Destination address/port',
laddr: 'Local address/port',
raddr: 'Remote address/port',
stopProcess: 'End',
stopProcessWarn: 'Are you sure you want to end this process (PID:{0})? ',
processName: 'Processname',
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/lang/modules/zh-Hant.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2505,8 +2505,8 @@ const message = {
env: '環境變量',
noenv: '無',
net: '網絡連接',
laddr: '源地址/端口',
raddr: '目標地址/端口',
laddr: '本地地址/端口',
raddr: '远程地址/端口',
stopProcess: '結束',
stopProcessWarn: '是否確定結束此進程 (PID:{0})？',
processName: '進程名稱',
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/lang/modules/zh.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2497,8 +2497,8 @@ const message = {
env: '环境变量',
noenv: '无',
net: '网络连接',
laddr: '源地址/端口',
raddr: '目标地址/端口',
laddr: '本地地址/端口',
raddr: '远程地址/端口',
stopProcess: '结束',
stopProcessWarn: '是否确定结束此进程 (PID:{0})？',
processName: '进程名称',
Expand Down
5 changes: 3 additions & 2 deletions frontend/src/views/website/runtime/php/extension-management/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,10 @@ const installExtension = async (row: Runtime.SupportExtension) => {
try {
await InstallPHPExtension(req);
taskLogRef.value.openWithTaskID(req.taskID);

} catch (error) {
} finally {
loading.value = false;
} catch (error) {}
}
});
};

Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 8, 2025

Choose a reason for hiding this comment

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

The code seems mostly correct, but here are a few improvements and suggestions:

  1. Exception Catches: The catch blocks can be merged to reduce redundancy. If only one error handling is needed after executing the promise chain (InstallPHPExtension(req)), then remove the second catch block.

  2. Variable Naming Consistency: Consider using PascalCase for variable names to improve readability, especially if they represent classes or objects.

  3. Promise Chaining Optimization: Ensure that each step of the promise chain is independent and does not rely on the outcome of another step. This helps in maintaining clean and modular code.

Here's an optimized version of the function:

const installExtension = async (row: Runtime.SupportExtension) => {
    try {
        await InstallPHPExtension(row.req); // Use PascalCase for req
        taskLogRef.value.openWithTaskID(row.req.taskID);
    } catch (error) {
        console.error('Error installing extension:', error);
    } finally {
        loading.value = false;
    }
};

Remember to adjust any other variables or functions in your project to maintain consistency with these guidelines.

Expand Down
Loading