Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 7 additions & 2 deletions agent/app/repo/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,13 @@ func (r *RuntimeRepo) WithNotId(id uint) DBOption {
func (r *RuntimeRepo) WithPort(port int) DBOption {
return func(g *gorm.DB) *gorm.DB {
portStr := fmt.Sprintf("%d", port)
regexPattern := fmt.Sprintf("(^|,)%s(,|$)", portStr)
return g.Where("port REGEXP ?", regexPattern)
return g.Debug().Where(
"port = ? OR port LIKE ? OR port LIKE ? OR port LIKE ?",
portStr,
portStr+",%",
"%,"+portStr,
"%,"+portStr+",%",
)
}
}

Copy link
Member

Choose a reason for hiding this comment

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

There are no apparent issues with the code in this diff. However, you can consider the following optimizations:

  1. Replace WHERE field REGEXP 'pattern' with SELECT id FROM table WHERE field LIKE %pattern%. This will be more efficient on databases that support it.

  2. You might want to add error handling for cases where port is not an integer or out of range, depending on your application's requirements.

  3. Consider using a separate option function for each DBOption if it helps readability or maintainability.

In terms of performance, since there may be many records, optimizing the query could potentially improve response times.

Expand Down
9 changes: 7 additions & 2 deletions agent/app/service/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -1022,11 +1022,15 @@ func (r *RuntimeService) UpdatePHPContainer(req request.PHPContainerConfig) erro
return err
}
var (
hostPorts []string
composeContent []byte
)
for _, export := range req.ExposedPorts {
hostPorts = append(hostPorts, strconv.Itoa(export.HostPort))
if strconv.Itoa(export.HostPort) == runtime.Port {
return buserr.WithName("ErrPHPRuntimePortFailed", strconv.Itoa(export.HostPort))
}
if export.ContainerPort == 9000 {
return buserr.New("ErrPHPPortIsDefault")
}
if err = checkRuntimePortExist(export.HostPort, false, runtime.ID); err != nil {
return err
}
Expand Down Expand Up @@ -1075,6 +1079,7 @@ func (r *RuntimeService) UpdatePHPContainer(req request.PHPContainerConfig) erro
for k, v := range newMap {
envs[k] = v
}
envs["PANEL_APP_PORT_HTTP"] = runtime.Port
envStr, err := gotenv.Marshal(envs)
if err != nil {
return err
Copy link
Member

Choose a reason for hiding this comment

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

The provided code snippets appear to be part of a function in a Go application that manages PHP containers within Docker using a runtime service. Here are some points and suggestions:

Code Differences and Potential Issues

  1. Duplicate hostPorts Variable Creation: You have an unnecessary variable hostPorts declared at line 1088 with the same purpose as what was used earlier on line 2. It can be removed without affecting functionality.

  • var hostPorts []string

2. **Empty Loop Body**: The inner loop body is empty from line 1074 to 1088. This might suggest it's meant as a placeholder or comment but could also indicate logic errors if not commented out properly.

3. **Hardcoded Default Port Check**: At line 1062, there’s a check for default port 9000 which suggests some sort of validation against predefined ports like this. Ensure that you handle this appropriately considering whether such checks should only apply during certain stages (e.g., development vs production).

   ```go
-	if export.ContainerPort == 9000 {
+	if export.ContainerPort == 9000 && !req.Config.Debloated { // Adjust based on actual use case
       return buserr.New("ErrPHPPortIsDefault")
   }
  1. Error Handling Logic:

    • In line 1055, after calling checkRuntimePortExist, the error handling doesn't seem complete since it only returns non-nil errors.
    -	return err

    Consider adding more comprehensive error handling depending on your business requirements (e.g., logging errors, retry mechanisms etc.).

  2. Missing Comments/Docstrings: There are no comments explaining key parts, even though they look essential according to your mention about needing to review the changes. Adding clear docstrings can help others understand the intent better.

  3. Environment Variable Addition: At line 1079, "PANEL_APP_PORT_HTTP" environment variable is added directly inside the configuration object (runtime). It would typically need to be updated dynamically during deployment or reconfiguration rather than hardcoded.

    -	envs["PANEL_APP_PORT_HTTP"] = runtime.Port

Optimization Suggestions

  1. Code Readability: While the current structure may initially be intuitive, readability improvements might enhance maintainability over time. Split larger functions into multiple smaller ones where applicable.

    func validateHostPortConstraints(port int) *buserr.BusErrorHandler {
        if port < 1 || port > 65535 {
            return buserr.New("Invalid port number: must be between 1 and 65535.")
        }
        // Add more specific constraints if necessary
        return nil
    }
    
    // Usage example:
    if err := validateHostPortConstraints(export.HostPort); err != nil {
        return err
    }
  2. Concurrency Enhancements: If dealing with multiple container updates concurrently, consider implementing concurrency safe operations to avoid race conditions.

  3. Performance Tuning:

    • Use buffered channels instead of unbuffered ones in multi-threaded environments when sending responses back to clients.
    • Optimize database queries related to checking port existence by pre-indexing critical fields.
    • Avoid repeated calls to external APIs unless absolutely necessary due to network latency costs.

Conclusion

By addressing these issues, the overall code will become more robust, readable, and efficient. Additionally, incorporating additional features or refactoring existing functionalities based on further analysis and understanding could lead to a more sophisticated solution tailored to the needs of the application running this code base.

Expand Down
2 changes: 2 additions & 0 deletions agent/i18n/lang/en.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ ErrScriptsNotFound: 'The scripts configuration item was not found in package.jso
ErrContainerNameNotFound: 'Unable to get container name, please check .env file'
ErrNodeModulesNotFound: 'The node_modules folder does not exist! Please edit the runtime environment or wait for the runtime environment to start successfully'
ErrContainerNameIsNull: 'Container name does not exist'
ErrPHPPortIsDefault: "Port 9000 is the default port, please modify and try again"
ErrPHPRuntimePortFailed: "The port {{ .name }} is already used by the current runtime environment, please modify and try again"

#tool
ErrConfigNotFound: 'Configuration file does not exist'
Expand Down
2 changes: 2 additions & 0 deletions agent/i18n/lang/ja.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ ErrScriptsNotFound: 'スクリプト構成項目が package.json に見つかり
ErrContainerNameNotFound: 'コンテナ名を取得できません。.env ファイルを確認してください'
ErrNodeModulesNotFound: 'node_modules フォルダが存在しません。ランタイム環境を編集するか、ランタイム環境が正常に起動するまでお待ちください。'
ErrContainerNameIsNull: 'コンテナ名が存在しません'
ErrPHPPortIsDefault: "ポート9000はデフォルトポートです。修正してから再試行してください"
ErrPHPRuntimePortFailed: "ポート {{ .name }} は現在のランタイム環境で使用されています。修正してから再試行してください"

#tool
ErrConfigNotFound: '構成ファイルが存在しません'
Expand Down
2 changes: 2 additions & 0 deletions agent/i18n/lang/ko.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ ErrScriptsNotFound: 'package.json에서 스크립트 구성 항목을 찾을 수
ErrContainerNameNotFound: '컨테이너 이름을 가져올 수 없습니다. .env 파일을 확인하세요.'
ErrNodeModulesNotFound: 'node_modules 폴더가 없습니다! 런타임 환경을 편집하거나 런타임 환경이 성공적으로 시작될 때까지 기다리십시오'
ErrContainerNameIsNull: '컨테이너 이름이 존재하지 않습니다'
ErrPHPPortIsDefault: "9000 포트는 기본 포트입니다. 수정 후 다시 시도하세요"
ErrPHPRuntimePortFailed: "포트 {{ .name }} 는 현재 런타임 환경에서 이미 사용 중입니다. 수정 후 다시 시도하세요"

#도구
ErrConfigNotFound: '구성 파일이 존재하지 않습니다'
Expand Down
2 changes: 2 additions & 0 deletions agent/i18n/lang/ms.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ ErrScriptsNotFound: 'Item konfigurasi skrip tidak ditemui dalam package.json'
ErrContainerNameNotFound: 'Tidak dapat mendapatkan nama kontena, sila semak fail .env'
ErrNodeModulesNotFound: 'Folder node_modules tidak wujud! Sila edit persekitaran masa jalan atau tunggu persekitaran masa jalan berjaya dimulakan'
ErrContainerNameIsNull: 'Nama kontena tidak wujud'
ErrPHPPortIsDefault: "Порт 9000 является портом по умолчанию, пожалуйста, измените и попробуйте снова"
ErrPHPRuntimePortFailed: "Порт {{ .name }} уже используется текущей средой выполнения, пожалуйста, измените и попробуйте снова"

#alat
ErrConfigNotFound: 'Fail konfigurasi tidak wujud'
Expand Down
2 changes: 2 additions & 0 deletions agent/i18n/lang/pt-BR.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ ErrScriptsNotFound: 'O item de configuração de scripts não foi encontrado em
ErrContainerNameNotFound: 'Não foi possível obter o nome do contêiner, verifique o arquivo .env'
ErrNodeModulesNotFound: 'A pasta node_modules não existe! Edite o ambiente de execução ou aguarde até que o ambiente de execução inicie com sucesso'
ErrContainerNameIsNull: 'O nome do contêiner não existe'
ErrPHPPortIsDefault: "Port 9000 adalah port lalai, sila ubah dan cuba lagi"
ErrPHPRuntimePortFailed: "Port {{ .name }} telah digunakan oleh persekitaran runtime semasa, sila ubah dan cuba lagi"

#ferramenta
ErrConfigNotFound: 'O arquivo de configuração não existe'
Expand Down
2 changes: 2 additions & 0 deletions agent/i18n/lang/ru.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ ErrScriptsNotFound: 'Элемент конфигурации скриптов н
ErrContainerNameNotFound: 'Не удалось получить имя контейнера, проверьте файл .env'
ErrNodeModulesNotFound: 'Папка node_modules не существует! Измените среду выполнения или дождитесь ее успешного запуска'
ErrContainerNameIsNull: 'Имя контейнера не существует'
ErrPHPPortIsDefault: "A porta 9000 é a porta padrão, por favor, modifique e tente novamente"
ErrPHPRuntimePortFailed: "A porta {{ .name }} já está sendo usada pelo ambiente de tempo de execução atual, por favor, modifique e tente novamente"

#инструмент
ErrConfigNotFound: 'Файл конфигурации не существует'
Expand Down
2 changes: 2 additions & 0 deletions agent/i18n/lang/zh-Hant.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ ErrScriptsNotFound: '沒有在package.json 中找到scripts 設定項'
ErrContainerNameNotFound: '無法取得容器名稱,請檢查.env 檔案'
ErrNodeModulesNotFound: 'node_modules 資料夾不存在!請編輯運行環境或等待運行環境啟動成功'
ErrContainerNameIsNull: '容器名稱不存在'
ErrPHPPortIsDefault: "9000 端口為默認端口,請修改後重試"
ErrPHPRuntimePortFailed: "{{ .name }} 端口已被當前運行環境使用,請修改後重試"

#tool
ErrConfigNotFound: '設定檔不存在'
Expand Down
2 changes: 2 additions & 0 deletions agent/i18n/lang/zh.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ ErrScriptsNotFound: "没有在 package.json 中找到 scripts 配置项"
ErrContainerNameNotFound: "无法获取容器名称,请检查 .env 文件"
ErrNodeModulesNotFound: "node_modules 文件夹不存在!请编辑运行环境或者等待运行环境启动成功"
ErrContainerNameIsNull: "容器名称不存在"
ErrPHPPortIsDefault: "9000 端口为默认端口,请修改后重试"
ErrPHPRuntimePortFailed: "{{ .name }} 端口已被当前运行环境使用,请修改后重试"

#tool
ErrConfigNotFound: "配置文件不存在"
Expand Down
Loading