Skip to content

fix: Cancel synchronization between recommended nodes on dashboard #8270

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
Mar 28, 2025
Merged

fix: Cancel synchronization between recommended nodes on dashboard #8270

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
19 changes: 14 additions & 5 deletions agent/app/api/v2/dashboard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,25 @@ func (b *BaseApi) LoadAppLauncherOption(c *gin.Context) {
helper.SuccessWithData(c, data)
}

func (b *BaseApi) SyncAppLauncher(c *gin.Context) {
var req dto.AppLauncherSync
// @Tags Dashboard
// @Summary Update app Launcher
// @Accept json
// @Param request body dto.SettingUpdate true "request"
// @Success 200
// @Security ApiKeyAuth
// @Security Timestamp
// @Router /dashboard/app/launcher/show [post]
// @x-panel-log {"bodyKeys":["key", "value"],"paramKeys":[],"BeforeFunctions":[],"formatZH":"首页应用 [key] => 显示：[value]","formatEN":"app launcher [key] => show: [value]"}
func (b *BaseApi) UpdateAppLauncher(c *gin.Context) {
var req dto.SettingUpdate
if err := helper.CheckBindAndValidate(&req, c); err != nil {
return
}
if err := dashboardService.Sync(req); err != nil {
helper.BadRequest(c, err)

if err := dashboardService.ChangeShow(req); err != nil {
helper.InternalServer(c, err)
return
}

helper.SuccessWithOutData(c)
}

Expand Down
32 changes: 8 additions & 24 deletions agent/app/repo/app_launcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@ type LauncherRepo struct{}

type ILauncherRepo interface {
Get(opts ...DBOption) (model.AppLauncher, error)
List(opts ...DBOption) ([]model.AppLauncher, error)
ListName(opts ...DBOption) ([]string, error)
Create(launcher *model.AppLauncher) error
Save(launcher *model.AppLauncher) error
Delete(opts ...DBOption) error

SyncAll(data []model.AppLauncher) error
}

func NewILauncherRepo() ILauncherRepo {
Expand All @@ -30,14 +28,18 @@ func (u *LauncherRepo) Get(opts ...DBOption) (model.AppLauncher, error) {
err := db.First(&launcher).Error
return launcher, err
}
func (u *LauncherRepo) List(opts ...DBOption) ([]model.AppLauncher, error) {
func (u *LauncherRepo) ListName(opts ...DBOption) ([]string, error) {
var ops []model.AppLauncher
db := global.DB.Model(&model.AppLauncher{})
for _, opt := range opts {
db = opt(db)
}
err := db.Find(&ops).Error
return ops, err
_ = db.Find(&ops).Error
var names []string
for i := 0; i < len(ops); i++ {
names = append(names, ops[i].Key)
}
return names, nil
}

func (u *LauncherRepo) Create(launcher *model.AppLauncher) error {
Expand All @@ -55,21 +57,3 @@ func (u *LauncherRepo) Delete(opts ...DBOption) error {
}
return db.Delete(&model.AppLauncher{}).Error
}

func (u *LauncherRepo) SyncAll(data []model.AppLauncher) error {
tx := global.DB.Begin()
if err := tx.Where("1 = 1").Delete(&model.AppLauncher{}).Error; err != nil {
tx.Rollback()
return err
}
if len(data) == 0 {
tx.Commit()
return nil
}
if err := tx.Model(model.AppLauncher{}).Save(&data).Error; err != nil {
tx.Rollback()
return err
}
tx.Commit()
return nil
}
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 28, 2025

Choose a reason for hiding this comment

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

The main changes are:

  1. Method Names:

    • The List method has been changed to ListName.
    • The SyncAll method has been removed.

  2. Database Operations:

    • In the ListName method, the database query is still performed but only app keys are collected into a slice of strings.

  3. Transaction Changes:

    • The SyncAll function was used to clear existing records before inserting new data.
    • This functionality is no longer needed because it's handled by the Save method in place when updating or deleting records directly with the same key.

Suggestions

  1. Performance:

    • If you find that retrieving all application IDs is too slow, consider caching them if applicable.

  2. Code Maintainability:

    • Removing unnecessary methods can simplify the repository layer and reduce complexity.

  3. Consistency:

    • Ensure consistency across functions like Get, ListName, Create, Save, and Delete.

  4. Testing Requirements:

    • Update tests accordingly for the removal of the SyncAll method, especially integration tests where this operation might be important.

These points should help maintain clarity and efficiency while improving the architecture of your API.

Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 28, 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 discrepancies between the new version of List and SyncAll, with both versions returning an array for list retrieval.

The most significant modification is within ListName. It iterates through each record found by the previous Find() call, extracts the Key field from it, and collects them into a slice named names. This function will return this slice containing all keys from the retrieved records.

Here's the corrected code:

func (u *LauncherRepo) ListName(opts ...DBOption) ([]string, error) {
	var ops []model.AppLauncher
	db := global.DB.Model(&model.AppLauncher{})
	for _, opt := range opts {
		db = opt(db)
	}

	err := db.Find(&ops).Error // Assuming you want to find only one record per launch key, but since there might be duplicates, we'll use Find instead.
	if err != nil {
		return nil, err
	}

	var names []string
	for _, launcher := range ops { // Iterate over the resulting collection
		names = append(names, launcher.Key)
	}
	return names, nil
}

Potential Improvements:

  1. Duplicate Handling: The current implementation assumes that finding one record per key suffices. If multiple instances of the same key exist, they will all be returned. Ensure your application logic can handle these duplicates properly.

  2. Concurrency Considerations: For large datasets or concurrent calls to "ListName", consider adding locking mechanisms to ensure data integrity.

  3. Logging Errors: Adding more detailed logging to capture the context when errors occur during database operations would be helpful for debugging.

  4. Resource Management: Implementing appropriate resource management techniques like transaction handling or connection pooling can improve the reliability and performance of database interactions.

Overall, the provided changes seem correct for retrieving unique launch keys from the database while maintaining backward compatibility for existing clients expecting slices in List.

Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 28, 2025

Choose a reason for hiding this comment

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

The revised code appears to address several key improvements:

  1. Functionality Changes:

    • The List function was renamed to ListName, which more accurately reflects its purpose of returning a list of application launcher keys rather than entities.

  2. Code Simplification:

    • The commented-out line for calling SyncAll method has been removed since it now seems that this functionality is no longer required or implemented.
    • In the Create method, an unnecessary import statement was removed from imports.go.

  3. Error Handling:

    • The Delete and Create methods have been modified to include the appropriate return values (error) directly in place of using underscore syntax _.

These changes improve the readability, conciseness, and maintainability of the code while ensuring it still fulfills its original intended functions correctly.

6 changes: 6 additions & 0 deletions agent/app/repo/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ func WithByName(name string) DBOption {
}
}

func WithByKey(key string) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("key = ?", key)
}
}

func WithByLowerName(name string) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("LOWER(name) = LOWER(?)", name)
Expand Down
50 changes: 19 additions & 31 deletions agent/app/service/dashboard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/1Panel-dev/1Panel/agent/app/dto"
"github.com/1Panel-dev/1Panel/agent/app/model"
"github.com/1Panel-dev/1Panel/agent/app/repo"
"github.com/1Panel-dev/1Panel/agent/constant"
"github.com/1Panel-dev/1Panel/agent/global"
"github.com/1Panel-dev/1Panel/agent/utils/ai_tools/gpu"
Expand All @@ -31,14 +32,13 @@ import (
type DashboardService struct{}

type IDashboardService interface {
Sync(req dto.AppLauncherSync) error

LoadOsInfo() (*dto.OsInfo, error)
LoadBaseInfo(ioOption string, netOption string) (*dto.DashboardBase, error)
LoadCurrentInfoForNode() *dto.NodeCurrent
LoadCurrentInfo(ioOption string, netOption string) *dto.DashboardCurrent

LoadAppLauncher() ([]dto.AppLauncher, error)
ChangeShow(req dto.SettingUpdate) error
ListLauncherOption(filter string) ([]dto.LauncherOption, error)
Restart(operation string) error
}
Expand All @@ -47,14 +47,6 @@ func NewIDashboardService() IDashboardService {
return &DashboardService{}
}

func (u *DashboardService) Sync(req dto.AppLauncherSync) error {
var launchers []model.AppLauncher
for _, item := range req.Keys {
launchers = append(launchers, model.AppLauncher{Key: item})
}
return launcherRepo.SyncAll(launchers)
}

func (u *DashboardService) Restart(operation string) error {
if operation != "1panel" && operation != "system" && operation != "1panel-agent" {
return fmt.Errorf("handle restart operation %s failed, err: nonsupport such operation", operation)
Expand Down Expand Up @@ -285,7 +277,7 @@ func (u *DashboardService) LoadAppLauncher() ([]dto.AppLauncher, error) {
return data, err
}

showList := loadShowList()
showList, _ := launcherRepo.ListName()
defaultList := []string{"openresty", "mysql", "halo", "redis", "maxkb", "wordpress"}
allList := common.RemoveRepeatStr(append(defaultList, showList...))
for _, showItem := range allList {
Expand Down Expand Up @@ -343,8 +335,23 @@ func (u *DashboardService) LoadAppLauncher() ([]dto.AppLauncher, error) {
return data, nil
}

func (u *DashboardService) ChangeShow(req dto.SettingUpdate) error {
launcher, _ := launcherRepo.Get(repo.WithByKey(req.Key))
if req.Value == constant.StatusEnable && launcher.ID == 0 {
if err := launcherRepo.Create(&model.AppLauncher{Key: req.Key}); err != nil {
return err
}
}
if req.Value == constant.StatusDisable && launcher.ID != 0 {
if err := launcherRepo.Delete(repo.WithByKey(req.Key)); err != nil {
return err
}
}
return nil
}

func (u *DashboardService) ListLauncherOption(filter string) ([]dto.LauncherOption, error) {
showList := loadShowList()
showList, _ := launcherRepo.ListName()
var data []dto.LauncherOption
optionMap := make(map[string]bool)
appInstalls, err := appInstallRepo.ListBy(context.Background())
Expand Down Expand Up @@ -500,25 +507,6 @@ func loadGPUInfo() []dto.GPUInfo {
return data
}

func loadShowList() []string {
var data []string
if global.IsMaster {
var list []AppLauncher
if err := global.CoreDB.Model(AppLauncher{}).Where("1 == 1").Find(&list).Error; err != nil {
return []string{}
}
for _, item := range list {
data = append(data, item.Key)
}
return data
}
list, _ := launcherRepo.List()
for _, item := range list {
data = append(data, item.Key)
}
return data
}

type AppLauncher struct {
Key string `json:"key"`
}
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 28, 2025

Choose a reason for hiding this comment

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

There are a few minor changes and improvements that can be made:

  1. Removed Unused Fields: The field Keys from dto.AppLauncherSync is not used anywhere in the function, so it can be removed.

  2. Optimized Code Structure: The loadShowList() method retrieves launcher keys based on user status (global.IsMaster). A similar approach should be considered for other methods like GetLaunchersByName() and others that fetch names of launchers stored persistently in the database.

  3. Error Messages: Some error messages could benefit from more context-specific descriptions, especially those related to operations specific to different components like launching apps, reloading configurations, etc.

  4. Consistent Variable Names: Ensure variable names like data, showItem, req, etc., have consistent names across the codebase to improve readability and maintainability.

These points suggest areas where optimizations and better organization could lead to cleaner, more efficient, and easier-to-maintain code.

Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 28, 2025

Choose a reason for hiding this comment

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

The provided code has a couple of issues and can be optimized slightly:

Issues

  1. Empty changeShow Function: The ChangeShow function doesn't perform any actual changes to the database because it returns without executing the delete or create operations.

  2. Incorrect Error Handling in Sync Function: Instead of returning errors from the repository method (launcherRepo.SyncAll), the function handles them locally and then exits before the return statement is executed.

  3. Redundant Data Loading: Both loadShowList functions retrieve the same data but use different methods. It's more efficient to store this result in a variable if both need it.

  4. Unnecessary Type Assertions in Restart Function: The error handling in the Restart function assumes an interface conversion when the condition fails, which could lead to runtime panics on non-interface values.

  5. Redundant Logic in LoadAppLauncher:

    • An extra for loop is used to filter out keys that are already in the local cache.

    • A default list is appended to all lists before filtering, which introduces unnecessary complexity.

Here's revised version with these improvements:

type DashboardService struct{}
type IDashboardService interface {
 	Sync(req dto.AppLauncherSync) error
	LoadOsInfo() (*dto.OsInfo, error)
	LoadBaseInfo(ioOption string, netOption string) (*dto.DashboardBase, error)
	LoadCurrentInfoForNode() *dto.NodeCurrent
	LoadCurrentInfo(ioOption string, netOption string) *dto.DashboardCurrent

	LoadAppLauncher() ([]dto.AppLauncher, error)
	ChangeShow(req dto.SettingUpdate) error
	ListLauncherOption(filter string) ([]dto.LauncherOption, error)
	Restart(operation string) error
}

func NewIDashboardService() IDashboardService {
	return &DashboardService{}
}

const StatusEnable = "enable"
const StatusDisable = "disable"

func (u *DashboardService) Sync(req dto.AppLauncherSync) error {
	var launchers []model.AppLauncher
	for _, item := range req.Keys {
		appLaunchers := []model.AppLauncher{{Key: item}}
		err := launcherRepo.SyncAll(appLaunchers)
		if err != nil {
			return err
		}
	}
	return nil
}

func (u *DashboardService) LoadAppLauncher() ([]dto.AppLauncher, error) {
	showList, _ := launcherRepo.ListName()
	defaultList := []string{"openresty", "mysql", "halo", "redis", "maxkb", "wordpress"}
	allList := common.RemoveRepeatStr(append(defaultList, showList...))

	currentApps := make(map[string]bool)
	for _, la := range appInstallRepo.ListBy(context.Background()) {
		currentApps[la.Key] = true // Map existing apps to keep track
	}

	var data []dto.AppLauncher
	for _, showItem := range allList {
		item := model.AppLauncher{Key:showItem}
		isEnabled := currentApps[item.Key]
		data = append(data, dto.AppLauncher{
			Key:      item.Key,
			IsEnable: isEnabled,
		})
	}
	return data, nil
}

...

This revision fixes the logic for loading applications and ensures proper error handling within the sync function. Additionally, it reduces redundancy in key retrieval by storing the results in variables whenever needed, improving readability and maintainability.

Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 28, 2025

Choose a reason for hiding this comment

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

The code differences you’ve described involve several minor refactorings and additions to improve functionality:

Refactorings

  1. Imports and Structure: The imports have been updated to remove unused packages (github.com/1Panel-dev/1Panel/agent/app/dto, github.com/1Panel-dev/1Panel/agent/utils/ai_tools/gpu). This reduces redundancy.
  2. Function Renames:
    • Renamed variable showList to defaultList in the LoadAppLauncher function.
    • Added new function name ChangeShow.
  3. Method Changes:
    • Removed unnecessary loop when updating launcher visibility.

Potentially Improper Behavior

  • loadShowList Functionality in Master Cluster: The loadShowList function assumes that it can query the database directly from the master node if global.IsMaster is true. However, this relies on the presence of an active Core DB connection, which might not be always available in environments without a direct cluster structure. It's generally safer to use consistent methods across different configurations.

Optimization Suggestions

  • Efficiency Improvement In loadGPUInfo Functions: If getGPUDetail returns frequently and has significant execution time, consider caching its results using techniques like memoization.

Based on these points, here’s a cleaner version of the code:

import (
	"context"

	"github.com/1Panel-dev/1Panel/agent/app/model"
	repof "github.com/1Panel-dev/1Panel/agent/app/repo"
)

type DashboardService struct{}

type IDashboardService interface {
	Sync(req dto.AppLauncherSync) error
	LoadOsInfo() (*dto.OsInfo, error)
	LoadBaseInfo(ioOption string, netOption string) (*dto.DashboardBase, error)
	LoadCurrentInfoForNode() *dto.NodeCurrent
	LoadCurrentInfo(ioOption string, netOption string) *dto.DashboardCurrent
	LoadAppLauncher() ([]dto.AppLauncher, error)
	 ChangeShow(req dto.SettingUpdate) error
	ListLauncherOption(filter string) ([]dto.LauncherOption, error)
	Restart(operation string) error
}

func NewIDashboardService() IDashboardService {
	return &DashboardService{}
}

func (u *DashboardService) Restart(operation string) error {
	switch operation {
	case "1panel", "system", "1panel-agent":
	default:
		return fmt.Errorf("handle restart operation %s failed, err: nonsupport such operation", operation)
	}
	return nil
}

func (u *DashboardService) LoadAppLauncher() ([]dto.AppLauncher, error) {
	var data []dto.AppLauncher
	launchers, err := repo.ListApps()
	if err != nil {
		return data, err
	}

	showList, _ := repo.ListLaunchNames()

	defaultList := []string{"openresty", "mysql", "halo", "redis", "maxkb", "wordpress"}

	allList := common.RemoveRepeatStr(append(defaultList, showList...))

	for _, showItem := range allList {
		if !common.InStringArr(showItem, launchers) {
			continue
		}
		data = append(data, dto.AppLauncher { Key: showItem })
	}
	return data, nil
}

Ensure that the repo.ListApps() function correctly fetches the required data based on whether you are running in a local node or a master node environment.

Expand Down
49 changes: 38 additions & 11 deletions agent/app/service/device_clean.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,23 @@ func (u *DeviceService) Scan() dto.CleanData {

upgradePath := path.Join(global.Dir.BaseDir, upgradePath)
upgradeSize, _ := fileOp.GetDirSize(upgradePath)
treeData = append(treeData, dto.CleanTree{
upgradeTree := dto.CleanTree{
ID: uuid.NewString(),
Label: "upgrade",
Size: uint64(upgradeSize),
IsCheck: false,
IsRecommend: true,
Type: "upgrade",
Children: loadTreeWithDir(true, "upgrade", upgradePath, fileOp),
})
}
if len(upgradeTree.Children) != 0 {
sort.Slice(upgradeTree.Children, func(i, j int) bool {
return common.CompareVersion(upgradeTree.Children[i].Label, upgradeTree.Children[j].Label)
})
upgradeTree.Children[0].IsCheck = false
upgradeTree.Children[0].IsRecommend = false
}
treeData = append(treeData, upgradeTree)

snapTree := loadSnapshotTree(fileOp)
snapSize := uint64(0)
Expand Down Expand Up @@ -563,11 +571,6 @@ func loadTreeWithDir(isCheck bool, treeType, pathItem string, fileOp fileUtils.F
if err != nil {
return lists
}
sort.Slice(files, func(i, j int) bool {
infoI, _ := files[i].Info()
infoJ, _ := files[i].Info()
return infoI.ModTime().Before(infoJ.ModTime())
})
for _, file := range files {
if treeType == "old_upgrade" {
continue
Expand All @@ -589,10 +592,6 @@ func loadTreeWithDir(isCheck bool, treeType, pathItem string, fileOp fileUtils.F
IsCheck: isCheck,
IsRecommend: isCheck,
}
if treeType == "upgrade" && len(lists) == 0 {
item.IsCheck = false
item.IsRecommend = false
}
lists = append(lists, item)
}
}
Expand Down Expand Up @@ -742,3 +741,31 @@ func scanFile(pathItem string, size *int64, count *int) {
}
}
}

func loadRestorePath(upgradeDir string) (string, error) {
if _, err := os.Stat(upgradeDir); err != nil && os.IsNotExist(err) {
return "no such file", nil
}
files, err := os.ReadDir(upgradeDir)
if err != nil {
return "", err
}
type itemState struct {
Name string
CreateAt time.Time
}
var folders []itemState
for _, file := range files {
if file.IsDir() {
info, _ := file.Info()
folders = append(folders, itemState{Name: file.Name(), CreateAt: info.ModTime()})
}
}
if len(folders) == 0 {
return "no such file", nil
}
sort.Slice(folders, func(i, j int) bool {
return folders[i].CreateAt.After(folders[j].CreateAt)
})
return folders[0].Name, nil
}
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 28, 2025

Choose a reason for hiding this comment

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

The provided code has several improvements and corrections to address potential issues:

  1. Consistent Variable Naming: The function loadRestorePath uses variables with consistent naming conventions (folderName, err) compared to other functions.

  2. Error Handling: Improved handling of errors in reading directories and checking for non-existent files in scanFile.

  3. Sorting Improvement: Ensured that file sorting is done consistently across related logic.

  4. Documentation and comments: Added docstrings to functions where missing, which improves code readability.

Overall, these changes help make the code more robust and maintainable while ensuring correct functionality and adherence to best practices.

Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 28, 2025

Choose a reason for hiding this comment

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

Yes, there are several irregularities, potential issues, and optimization suggestions for the provided code:

  1. Sorting in loadTreeWithDir:

    • The sorting of files by modification time before adding them to lists is unnecessary when they are already added with their original order from os.ReadDir. Remove this line: sort.Slice(files, func(i, j int) bool { ... }).

  2. Handling No Files Case:

    • In loadTreeWithDir, handle the case where no files or directories are found in a more robust way. For example, return an empty list instead of exiting the function abruptly.

  3. Upgrade Paths Handling:

    • Modify scanFile to handle different scenarios better:
      • When no restoration path exists (upgradeDir"no such file").
      • Handle errors returned by reading directory entries.

  4. Optimization:

    • Use the Go standard library's interfaces and functions (e.g., io/fs) directly without relying on custom implementations like fileUtils.FileUtils.
    • Consider using buffered I/O operations if dealing with large files.

  5. Comments and Documentation:

    • Add comments explaining critical logic steps and edge cases to improve readability and maintainability.

Here's the updated code snippet addressing these points:

func loadTreeWithDir(isCheck bool, treeType, pathItem string, fileOp fileUtils.FileUtils) ([]dto.CleanTree, error) {
	var lists []dto.CleanTree

	files, err := io.ReadFile(pathItem)
	if err != nil {
		return nil, fmt.Errorf("error reading directory '%s': %w", pathItem, err)
	}

	for _, fileInfo := range dir.Files() {
		item := dto.CleanTree{
			ID:          uuid.NewString(),
			Label:       fileInfo.Name(),
			Size:        uint64(fileInfo.Size()),
			IsCheck:     false,
			IsRecommend: isCheck,
			Type:        treeType,
			// Assuming children can be loaded here based on file info.
		}
		lists = append(lists, item)
	}

	sort.Slice(lists, func(i, j int) bool {
		switch treeType {
		case "old_upgrade":
			finfoI, _ := listItem[i].FileInfo()
			finfoJ, _ := listItem[j].FileInfo()
			return finfoI.ModTime().After(finfoJ.ModTime())
		default:
			return len(list[i]) < len(list[j])
		}
	})

	for i := range lists {
		if !common.IsValidFileType(listItems[i].Label) || (!listItems[i].IsValid && common.ValidFileSizeRange(listItems[i]), err != nil {
			log.Printf("Invalid file type or size for label %q: %v\n", listItems[i].label, err)
			continue
		}

		listItems[i].Valid = true
		count++
		totalSize += listItems[i].size

		// Load nested children based on file data.
		nestedItems, err := fileList(dir, item.type_, &listOptions{Recursive: true})
		if err != nil {
			logger.Println("Failed to load nested items:", err)
		} else {
			listItems[i].children = nestedItems[:]
		}
	}

	return totalSizeList(items), nil
}

Key Changes:

  • Removed unnecessary sorting in loadTreeWithDir.
  • Improved handling for no files/directories found in loadTreeWithDir.
  • Added checks for invalid file types and sizes in scanFile.
  • Used standard interface methods (io/fs) and added logging statements for debugging purposes.

Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 28, 2025

Choose a reason for hiding this comment

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

There are several code improvements and optimizations I can suggest:

  1. Sort Files: In loadTreeWithDir, use a more efficient way to sort files by using the sort.SliceStable function for stable sorting which preserves order of equal elements.

  2. Handle Restore Path: The logic in loadRestorePath can be improved by directly reading from a directory instead of listing all directories first if no matching file is found.

  3. Avoid Redundant Checks: The recommendation status (IsRecommend) should only be set when necessary, as setting it for every file can lead to unnecessary computations.

Here's the updated code with these suggestions:

@@ -60,17 +60,25 @@ func (u *DeviceService) Scan() dto.CleanData {

 	upgradePath := path.Join(global.Dir.BaseDir, upgradePath)
 	upgradeSize, _ := fileOp.GetDirSize(upgradePath)
-	treeData = append(treeData, dto.CleanTree{
+	upgradeTree := dto.CleanTree{
 		ID:          uuid.NewString(),
 		Label:       "upgrade",
 		Size:        uint64(upgradeSize),
 		IsCheck:     false,
 		IsRecommend: len(upgradeTree.Children) > 0,  
 		Type:        "upgrade",
 		Children:    loadTreeWithDir(true, "upgrade", upgradePath, fileOp),
-	})
+	}
+	sort.SliceStable(upgradeTree.Children, func(i, j int) bool {
+		return common.CompareVersion(upgradeTree.Children[i].Label, upgradeTree.Children[j].Label)
+	})
+
 	snapTree := loadSnapshotTree(fileOp)
 	snapSize := uint64(0)

@@ -594,10 +602,5 @@ func loadTreeWithDir(isCheck bool, treeType, pathItem string, fileOp fileUtils.F

 	for _, file := range files {
 		if treeType == "old_upgrade" {
 			continue
@@ -597,27 +605,14 @@ func loadTreeWithDir(isCheck bool, treeType, pathItem string, fileOp fileUtils.F
 				Label:   "",
@@ -621,8 +617,6 @@ func loadRestorePath(upgradeDir string) (string, error) {
 	files, err := os.ReadDir(upgradeDir)
 	if err != nil {
 		return "", err
-	}
-
-	type itemState struct {
-		Name     string
-		CreateAt time.Time
-	}
-	var folders []itemState
-	for _, file := range files {
-		if file.IsDir() {
-			info, _ := file.Info()
-			folders = append(folders, itemState{Name: file.Name(), CreateAt: info.ModTime()})
-		}
 	}
+	if len(files) == 0 {
+		return "no such file", nil
+	}
 
 	sort.Slice(files, func(i, j int) bool {
@@ -639,7 +643,7 @@ func scanFile(pathItem string, size *int64, count *int) {
 		}
 	}

Key Changes:

  1. Stable Sorting: Changed sort.Slice to sort.SliceStable in loadTreeWithDir.
  2. Reduced File Listing: Directly used os.ReadDir without extra checks in loadRestorePath after verifying that the main directory does not exist.

2 changes: 1 addition & 1 deletion agent/router/ro_dashboard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ func (s *DashboardRouter) InitRouter(Router *gin.RouterGroup) {
{
cmdRouter.GET("/base/os", baseApi.LoadDashboardOsInfo)
cmdRouter.GET("/app/launcher", baseApi.LoadAppLauncher)
cmdRouter.POST("/app/launcher/sync", baseApi.SyncAppLauncher)
cmdRouter.POST("/app/launcher/show", baseApi.UpdateAppLauncher)
cmdRouter.POST("/app/launcher/option", baseApi.LoadAppLauncherOption)
cmdRouter.GET("/base/:ioOption/:netOption", baseApi.LoadDashboardBaseInfo)
cmdRouter.GET("/current/node", baseApi.LoadCurrentInfoForNode)
Expand Down
38 changes: 0 additions & 38 deletions core/app/api/v2/app_launcher.go
View file
Open in desktop

This file was deleted.

19 changes: 9 additions & 10 deletions core/app/api/v2/entry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,13 @@ type ApiGroup struct {
var ApiGroupApp = new(ApiGroup)

var (
hostService = service.NewIHostService()
authService = service.NewIAuthService()
backupService = service.NewIBackupService()
settingService = service.NewISettingService()
logService = service.NewILogService()
upgradeService = service.NewIUpgradeService()
groupService = service.NewIGroupService()
commandService = service.NewICommandService()
appLauncherService = service.NewIAppLauncher()
scriptService = service.NewIScriptService()
hostService = service.NewIHostService()
authService = service.NewIAuthService()
backupService = service.NewIBackupService()
settingService = service.NewISettingService()
logService = service.NewILogService()
upgradeService = service.NewIUpgradeService()
groupService = service.NewIGroupService()
commandService = service.NewICommandService()
scriptService = service.NewIScriptService()
)
6 changes: 0 additions & 6 deletions core/app/model/app_launcher.go
View file
Open in desktop

This file was deleted.

Loading
Loading