fix: Fix the problem of abnormal acquisition of user-info

[ssongliu]

Refs #8920

[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-sigs/prow repository.

[wanghe-fit2cloud]

There is no significant issue with the provided code. The changes made to add "core.db" as an argument to loadDBConn() do not affect the functionality or logic of the commands. Each command initializes a new database connection using loadDBConn(), which takes a string representing the database filename as an optional parameter, defaulting to "db.sqlite".

The addition of "core.db" ensures clarity about where the core configuration file might be located relative to other databases used by the application. This change does not introduce any performance overhead since loading multiple databases would likely lead to caching and efficient access.

Therefore, the existing code is correct and can remain unchanged unless further context suggests it should handle different scenarios based on these filenames differently.

[wanghe-fit2cloud]

Code Differences Check

The provided diff output shows changes to the userinfoCmd function. Here are the key points to review:

1. Database Connection Paths:

   • Line 22: Initially, it uses "" instead of '. This seems like a typo.
   • Lines 10 to 22 (not included in the diff but related): It adds two additional database connection paths "core.db" and "agent.db", using the same function call.

2. User Agent Database Loading:

   • Lines 25-29 load a new database (db) and handle an error if the loading fails.

3. Agent Database Loading:

   • Lines 32-37 load another database (agentDB) and also handle an error if this operation fails.

4. Key Retrieval Logic:

   • Lines 45 and below use the newly loaded agentDB to retrieve certain settings instead of the original db.

Potential Issues and Improvements

1. Typographical Mistake in Diff:

   • Remove the unnecessary single quotes from line 22 to correct the typo.

2. Code Readability and Maintainability:

   • Consider splitting the code into smaller functions or methods to improve readability. For example, separate steps for connecting to databases, retrieving settings, and handling errors might make maintenance easier.

3. Error Handling Consistency:

   • Ensure that all error handling operations are consistent and clear. If there's a significant difference in what kind of errors each branch handles, consider centralizing these behaviors.

4. Comments and Docstrings:

   • Add comments to explain complex logic segments or sections, especially where different database connections are used, to aid understanding.

5. Logging Level:

   • Update error messages to include more specific details about which setting retrieval failed, such as "Failed to set user info because fetching server port from 'systemip' was unsuccessful."

Here’s a suggested refactored version incorporating some of these improvements with minimal changes:

func LoadSettingsAndConnections(ctx context.Context, cmd *cobra.Command) (*gorm.DB, *gorm.DB, error) {
	var agentDbConfig struct {
		DBName string `mapstructure:"name"`
	}

	err := config.UnmarshalKey(config.KeyAgentDB, &agentDbConfig)
	if err != nil {
		return nil, nil, fmt.Errorf("unmarshaling agent DB config: %v", err)
	}

	customDBOptions, ok := ctx.Value(context.CTXKeyDB).(map[string]string)
	if !ok {
		return nil, nil, errors.New("DB options not found in context")
	}

	db, err := openGormDatabases(customDBOptions["core"], customDBOptions["agent"])
	if err != nil {
		return nil, nil, fmt.Errorf("open databases failed: %v", err)
	}

	db.SetLogger(sql.LoggerAdapter(logrus.StandardLogger()))
	agentDB.SetLogger(sql.LoggerAdapter(logrus.StandardLogger()))

	// Retrieve and assign values to settings here...
	
	return db, agentDB, nil
}

This approach consolidates the database opening process and makes error handling uniform across functions, improving overall maintainability and robustness.

[wanghe-fit2cloud]

The provided patch is mostly correct, but there are a few areas that could be improved:

1. Duplicated Code: There are three identical blocks where the database connection is attempted to be loaded with loadDBConn. This can be optimized by using function reusability.

2. Potential Issues: If something goes awry during the loading of the database connection more than once, it might lead to redundant error handling and additional complexity.

Here's an optimized version of the code:

 func setupDatabaseConnection(dbPath string) (*db.DB, error) {
 	db, err := db.NewSQLClient(dbPath)
 	if err != nil {
 		return nil, fmt.Errorf(i18n.GetMsgWithMapForCmd("FailedToLoadConfigFile", map[string]interface{}{
 			"file": "core.db",
 			"error": err.Error(),
 		}))
 	}
 	return db, nil
 }

func username() {
 	// Some other logic...

-	db, err := loadDBConn()
+	connErrPrefix := "Failed to establish a database connection"
+	db, connErr := setupDatabaseConnection("core.db")

 	// Error handling...
}

func password() {
	// Similar logic...

-	db, err := loadDBConn()
+	db, connErr := setupDatabaseConnection("core.db")

Explanation:

1. SetupFunction: Introduced a new function setupDatabaseConnection that encapsulates the database connection logic. It uses a custom prefix for error messages to make them distinct.

   func setupDatabaseConnection(dbPath string) (*db.DB, error) {...}

2. Use in Functions: Instead of calling loadDBConn directly within each function, use the connectFn helper function which calls setupDatabaseConnection.

3. Single Point for Connection Errors: By doing this, we reduce redundancy in error handling and improve maintainability of the codebase.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[wanghe-fit2cloud]

/lgtm

[wanghe-fit2cloud]

/approve

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wanghe-fit2cloud

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [wanghe-fit2cloud]

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