Skip to content

feat(common): Optimize commonRepo Invocation #7621

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
Jan 2, 2025
Merged

feat(common): Optimize commonRepo Invocation #7621

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
62 changes: 18 additions & 44 deletions agent/app/repo/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,62 +12,34 @@ import (

type DBOption func(*gorm.DB) *gorm.DB

type ICommonRepo interface {
WithByID(id uint) DBOption
WithByIDs(ids []uint) DBOption
WithByName(name string) DBOption
WithByNames(names []string) DBOption
WithByLikeName(name string) DBOption
WithByDetailName(detailName string) DBOption

WithByFrom(from string) DBOption
WithByType(tp string) DBOption
WithTypes(types []string) DBOption
WithByStatus(status string) DBOption

WithOrderBy(orderStr string) DBOption
WithOrderRuleBy(orderBy, order string) DBOption

WithByDate(startTime, endTime time.Time) DBOption
WithByCreatedAt(startTime, endTime time.Time) DBOption
}

type CommonRepo struct{}

func NewCommonRepo() ICommonRepo {
return &CommonRepo{}
}

func (c *CommonRepo) WithByID(id uint) DBOption {
func WithByID(id uint) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("id = ?", id)
}
}
func (c *CommonRepo) WithByIDs(ids []uint) DBOption {

func WithByIDs(ids []uint) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("id in (?)", ids)
}
}

func (c *CommonRepo) WithByName(name string) DBOption {
func WithByName(name string) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("name = ?", name)
}
}
func (c *CommonRepo) WithByNames(names []string) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("name in (?)", names)
}
}
func (c *CommonRepo) WithByLikeName(name string) DBOption {

func WithByLikeName(name string) DBOption {
return func(g *gorm.DB) *gorm.DB {
if len(name) == 0 {
return g
}
return g.Where("name like ?", "%"+name+"%")
}
}
func (c *CommonRepo) WithByDetailName(detailName string) DBOption {

func WithByDetailName(detailName string) DBOption {
return func(g *gorm.DB) *gorm.DB {
if len(detailName) == 0 {
return g
Expand All @@ -76,51 +48,53 @@ func (c *CommonRepo) WithByDetailName(detailName string) DBOption {
}
}

func (c *CommonRepo) WithByType(tp string) DBOption {
func WithByType(tp string) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("type = ?", tp)
}
}
func (c *CommonRepo) WithTypes(types []string) DBOption {

func WithTypes(types []string) DBOption {
return func(db *gorm.DB) *gorm.DB {
return db.Where("type in (?)", types)
}
}
func (c *CommonRepo) WithByStatus(status string) DBOption {

func WithByStatus(status string) DBOption {
return func(g *gorm.DB) *gorm.DB {
if len(status) == 0 {
return g
}
return g.Where("status = ?", status)
}
}
func (c *CommonRepo) WithByFrom(from string) DBOption {
func WithByFrom(from string) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("`from` = ?", from)
}
}

func (c *CommonRepo) WithByDate(startTime, endTime time.Time) DBOption {
func WithByDate(startTime, endTime time.Time) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("start_time > ? AND start_time < ?", startTime, endTime)
}
}

func (c *CommonRepo) WithByCreatedAt(startTime, endTime time.Time) DBOption {
func WithByCreatedAt(startTime, endTime time.Time) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("created_at > ? AND created_at < ?", startTime, endTime)
}
}

func (c *CommonRepo) WithOrderBy(orderStr string) DBOption {
func WithOrderBy(orderStr string) DBOption {
if orderStr == "createdAt" {
orderStr = "created_at"
}
return func(g *gorm.DB) *gorm.DB {
return g.Order(orderStr)
}
}
func (c *CommonRepo) WithOrderRuleBy(orderBy, order string) DBOption {
func WithOrderRuleBy(orderBy, order string) DBOption {
if orderBy == "createdAt" {
orderBy = "created_at"
}
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Jan 2, 2025

Choose a reason for hiding this comment

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

This is a code snippet related to an instance of interface 'ICommonRepo' from the Go programming language. The code introduces a new version of CommonRepo, with different implementations of various methods.

First off, it creates its own repository struct named CommonRepo and also implements NewCommonRepo function that returns a pointer to CommonRepo.

Then within this structure, there is extensive implementation details of CRUD (Create Read Update Delete) operations including specific ones like ByID(), WithByID() , etc. These functions are used in the scope of the CommonRepo package itself.

The key points of interest here:

  • Method WithOrderBy takes in an order string ('createdAt', 'updatedAt') if specified as default but uses created_at otherwise.
  • There are no explicit use-cases provided for all these implemented method calls so further investigation may be required based on what they might do in context.

Given its nature and purpose being mostly internal API usage across components of larger application architecture, this could possibly serve multiple roles such as data access layer validation rules or more specialized database querying techniques.

For optimization/irregularity checks, one thing could focus on would be how well defined are each *gorm.DB query parameter handling strategies?

Potential areas require attention could include ensuring these implement proper safety and security practices considering typical go/rust style conventions when working with external APIs, especially given their direct call pattern in the current implementation.

There's no particular issue flagged out directly; but due to its complexity and potential wide impact beyond immediate area, any major change should probably undergo some kind of rigorous testing before release!

Expand Down
5 changes: 0 additions & 5 deletions agent/app/repo/entry.go
View file
Open in desktop

This file was deleted.

12 changes: 9 additions & 3 deletions agent/app/repo/task.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package repo

import (
"context"

"github.com/1Panel-dev/1Panel/agent/constant"
"github.com/1Panel-dev/1Panel/agent/global"

Expand All @@ -24,6 +23,7 @@ type ITaskRepo interface {
WithByID(id string) DBOption
WithResourceID(id uint) DBOption
WithOperate(taskOperate string) DBOption
WithByStatus(status string) DBOption
}

func NewITaskRepo() ITaskRepo {
Expand Down Expand Up @@ -67,6 +67,12 @@ func (t TaskRepo) WithResourceID(id uint) DBOption {
}
}

func (t TaskRepo) WithByStatus(status string) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("status = ?", status)
}
}

func (t TaskRepo) Save(ctx context.Context, task *model.Task) error {
return getTaskTx(ctx).Save(&task).Error
}
Expand Down Expand Up @@ -94,11 +100,11 @@ func (t TaskRepo) Update(ctx context.Context, task *model.Task) error {
}

func (t TaskRepo) UpdateRunningTaskToFailed() error {
return getTaskDb(commonRepo.WithByStatus(constant.StatusExecuting)).Model(&model.Task{}).Updates(map[string]interface{}{"status": constant.StatusFailed, "error_msg": "1Panel restart causes failure"}).Error
return getTaskDb(t.WithByStatus(constant.StatusExecuting)).Model(&model.Task{}).Updates(map[string]interface{}{"status": constant.StatusFailed, "error_msg": "1Panel restart causes failure"}).Error
}

func (t TaskRepo) CountExecutingTask() (int64, error) {
var count int64
err := getTaskDb(commonRepo.WithByStatus(constant.StatusExecuting)).Model(&model.Task{}).Count(&count).Error
err := getTaskDb(t.WithByStatus(constant.StatusExecuting)).Model(&model.Task{}).Count(&count).Error
return count, err
}
20 changes: 10 additions & 10 deletions agent/app/service/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (a AppService) PageApp(req request.AppSearch) (interface{}, error) {
for _, t := range appTags {
appIds = append(appIds, t.AppId)
}
opts = append(opts, commonRepo.WithByIDs(appIds))
opts = append(opts, repo.WithByIDs(appIds))
}
var res response.AppRes

Expand Down Expand Up @@ -203,7 +203,7 @@ func (a AppService) GetAppDetail(appID uint, version, appType string) (response.
appDetailDTO.Enable = true

if appType == "runtime" {
app, err := appRepo.GetFirst(commonRepo.WithByID(appID))
app, err := appRepo.GetFirst(repo.WithByID(appID))
if err != nil {
return appDetailDTO, err
}
Expand Down Expand Up @@ -274,7 +274,7 @@ func (a AppService) GetAppDetail(appID uint, version, appType string) (response.

appDetailDTO.HostMode = isHostModel(appDetailDTO.DockerCompose)

app, err := appRepo.GetFirst(commonRepo.WithByID(detail.AppId))
app, err := appRepo.GetFirst(repo.WithByID(detail.AppId))
if err != nil {
return appDetailDTO, err
}
Expand All @@ -288,7 +288,7 @@ func (a AppService) GetAppDetail(appID uint, version, appType string) (response.
}
func (a AppService) GetAppDetailByID(id uint) (*response.AppDetailDTO, error) {
res := &response.AppDetailDTO{}
appDetail, err := appDetailRepo.GetFirst(commonRepo.WithByID(id))
appDetail, err := appDetailRepo.GetFirst(repo.WithByID(id))
if err != nil {
return nil, err
}
Expand All @@ -309,7 +309,7 @@ func (a AppService) GetIgnoredApp() ([]response.IgnoredApp, error) {
return res, nil
}
for _, detail := range details {
app, err := appRepo.GetFirst(commonRepo.WithByID(detail.AppId))
app, err := appRepo.GetFirst(repo.WithByID(detail.AppId))
if err != nil {
return nil, err
}
Expand All @@ -328,7 +328,7 @@ func (a AppService) Install(req request.AppInstallCreate) (appInstall *model.App
err = buserr.WithDetail(constant.Err1PanelNetworkFailed, err.Error(), nil)
return
}
if list, _ := appInstallRepo.ListBy(commonRepo.WithByName(req.Name)); len(list) > 0 {
if list, _ := appInstallRepo.ListBy(repo.WithByName(req.Name)); len(list) > 0 {
err = buserr.New(constant.ErrAppNameExist)
return
}
Expand All @@ -338,16 +338,16 @@ func (a AppService) Install(req request.AppInstallCreate) (appInstall *model.App
appDetail model.AppDetail
app model.App
)
appDetail, err = appDetailRepo.GetFirst(commonRepo.WithByID(req.AppDetailId))
appDetail, err = appDetailRepo.GetFirst(repo.WithByID(req.AppDetailId))
if err != nil {
return
}
app, err = appRepo.GetFirst(commonRepo.WithByID(appDetail.AppId))
app, err = appRepo.GetFirst(repo.WithByID(appDetail.AppId))
if err != nil {
return
}
if DatabaseKeys[app.Key] > 0 {
if existDatabases, _ := databaseRepo.GetList(commonRepo.WithByName(req.Name)); len(existDatabases) > 0 {
if existDatabases, _ := databaseRepo.GetList(repo.WithByName(req.Name)); len(existDatabases) > 0 {
err = buserr.New(constant.ErrRemoteExist)
return
}
Expand Down Expand Up @@ -462,7 +462,7 @@ func (a AppService) Install(req request.AppInstallCreate) (appInstall *model.App
appInstall.DockerCompose = string(composeByte)

if hostName, ok := req.Params["PANEL_DB_HOST"]; ok {
database, _ := databaseRepo.Get(commonRepo.WithByName(hostName.(string)))
database, _ := databaseRepo.Get(repo.WithByName(hostName.(string)))
if !reflect.DeepEqual(database, model.Database{}) {
req.Params["PANEL_DB_HOST"] = database.Address
req.Params["PANEL_DB_PORT"] = database.Port
Expand Down
Loading
Loading