feat(website): fix issues with create website with ftp failed

[zhengkunwang223]

No description provided.

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from zhengkunwang223. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[wanghe-fit2cloud]

The provided code has some areas that can be reviewed for better organization, clarity, and performance.

Code Snippet 1

if len(create.FtpUser) != 0 && len(create.FtpPassword) != 0 {
    createFtpUser := func(t *task.Task) error {
        indexDir := GetSitePath(*website, SiteIndexDir)
        itemID, err := NewIFtpService().Create(dto.FtpCreate{User: create.FtpUser, Password: create.FtpPassword, Path: indexDir})
        if err != nil {
            t.Log(fmt.Sprintf("create ftp for website failed, err: %v", err))
        }
        website.FtpID = itemID
        return nil
    }

    deleteFtpUser := func(t *task.Task) {
        if website.FtpID > 0 {
            if err = NewIFtpService().Delete(dto.BatchDeleteReq{Ids: []uint{website.FtpID}}); err != nil {
                t.Log(err.Error())
            }
        }
    }

    createTask.AddSubTask(i18n.GetWithName("ConfigFTP", create.FtpUser), createFtpUser, deleteFtpUser)
}

Recommendations:

• Function Simplification: Consider combining createFtpUser and deleteFtpUser into a single function that handles both operations simultaneously.

func ensureFtpUserExists(t context.Context) func(ctx context.Context) error {
    return func(ctx context.Context) error {
        user := dto.FtpCreate{
            User:   create.FtpUser,
            Password: create.FtpPassword,
            Path:     GetSitePath(*website, SiteIndexDir),
        }
        
        itemID, err := NewIFtpService().Create(user)
        if err != nil {
            return err
        }
        
        website.FtpID = itemID
        
        // Cleanup on failure
        defer cleanupFunc(itemID)

        return nil
    }
}

func cleanupFunc(id uint) {
    if id > 0 {
        if err := NewIFtpService().Delete(dto.BatchDeleteReq{Ids: []uint{id}}); err != nil {
            log.Printf("Unable to clean up FTP resource: %v\n", err)
        }
    }
}

• Context Use: It's generally recommended to use contexts instead of task execution directly inside tasks. Ensure all functions used accept and handle contexts properly.

2. Log Formatting: Improve logging formatting to ensure consistency across different environments (t.Log() should be replaced with consistent logging methods according to specific project standards).

Code Snippet 2

if len(create.FtpUser) != 0 && len(create.FtpPassword) != 0 {
    createFtpUser := func(t *task.Task) error {
        ...
    }

    createTask.AddSubTask(i18n.GetWithName("ConfigFTP", create.FtpUser), createFtpUser)
}

No significant changes here but the pattern is similar to snippet 1.

General Recommendations

1. Consistent Structure: Encourage adhering to a standard structure like one of the popular Go coding patterns such as interfaces, structs, or modules.

2. Unit Test Coverage: Always test edge cases including empty strings, null values, etc., especially where new code introduces logic flow.

3. Code readability: Avoid deeply nested blocks unless absolutely necessary. Refactor long functions/functions with many parameters if they make it hard to understand at glance.

4. Optimization Tips:

   • Minimize repeated calls to expensive operation like database queries within loops unless profiling indicates otherwise.
   • Utilize buffered channels or go-routines efficiently to perform parallel operations wherever feasible and safe.

5. Avoid Global Variables: If you need shared data between functions, consider using package-level variables cautiously after careful analysis to avoid race conditions.

6. Linter Checks: Regularly run linters (like golint, gosec) to identify and fix style inconsistencies before submitting for review.

Overall Feedback: The code seems well-structured except for minor inefficiencies and inconsistent logging practices which could impact maintainability and debugging effort. Suggestions focus on improving code cleanliness, consistency, and efficiency.

[wanghe-fit2cloud]

There is an issue with the logic inside the if block before saving the runtime status to running if no errors occur up to that point. Specifically:

1. The function should not call _runtimeRepo.Save(runtime) inside both branches of the conditional statement (before and after the installation command) since it may overwrite changes made in one branch.

2. After processing the installation command, there might be some additional cleanup needed (depending on what compose.DownAndUp() does), but this part needs clarification based on the actual implementation.

For optimization, consider moving any operations unrelated to runtime state updates above the conditional statements to reduce redundancy. Additionally, ensure error handling when calling downAndUp() since downgrading and upgrading the container can fail as well.

[wanghe-fit2cloud]

There are several areas where this code can be improved:

1. Remove Unused Code

• The createRef is not used anywhere after its initialization.

// remove this line: const createRef = ref();

Additionally, the unused methods operateAuthConfig, getAuthConfig, and possibly others might need to be removed as well if they are not referenced elsewhere.

2. Simplify Variable Declarations

Declare variables more clearly when using ref() or even directly assigning values during setup.

const data = ref([]); // instead of let data = [];

const loading = ref(false);

...

onMounted(() => {
    loadData();
});

3. Refactor Complex Logic

If the complex logic related to checking conditions should only run under certain circumstances, consider wrapping it in a separate function with conditional checks within.

function handleShowPath(websiteType) {
  showPath.value = websiteType !== 'proxy' && websiteType !== 'deployment';
}

...
handleShowPath(siteDetail.type);

This makes the code cleaner and easier to maintain.

4. Add More TypeScript Type Checking

Ensure that types are correctly defined and used throughout the component.

interface WebSiteType { /** Define your interface here */ }

const siteDetail: WebSiteType = computed(() => {/* logic to compute siteDetail*/ });

... other similar uses.

5. Ensure Error Handling

Include error handling for APIs like getWebsite and getPathData.

async function loadWebsites() {
   try {
      const response = await getWebsite(id.value);
      pathData.value = response.data;
   } catch (error) {
      console.error("Error fetching websites", error.message);
   }
}
loadWebsites();

These recommendations will help streamline the codebase by removing unnecessary parts, simplifying variable declarations, improving modularity, adding type definitions explicitly, and ensuring robustness through proper error handling.

[sonarqubecloud]

Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE