likyh commented on code in PR #3986:
URL: 
https://github.com/apache/incubator-devlake/pull/3986#discussion_r1053931717


##########
services/project.go:
##########
@@ -22,286 +22,275 @@ import (
 
        "github.com/apache/incubator-devlake/errors"
        "github.com/apache/incubator-devlake/models"
-       "github.com/apache/incubator-devlake/plugins/core"
+       "github.com/apache/incubator-devlake/models/domainlayer/crossdomain"
+       "github.com/apache/incubator-devlake/plugins/core/dal"
        "github.com/apache/incubator-devlake/plugins/helper"
 )
 
 // ProjectQuery used to query projects as the api project input
 type ProjectQuery struct {
-       Page     int `form:"page"`
-       PageSize int `form:"pageSize"`
+       Pagination
 }
 
-// CreateProject accepts a project instance and insert it to database
-func CreateProject(project *models.Project) errors.Error {
-       if project.Name == "" {
-               return errors.Default.New("can not use empty name for project")
+// GetProjects returns a paginated list of Projects based on `query`
+func GetProjects(query *ProjectQuery) ([]*models.Project, int64, errors.Error) 
{
+       // verify input
+       if err := VerifyStruct(query); err != nil {
+               return nil, 0, err
        }
-
-       /*project, err := encryptProject(project)
-       if err != nil {
-               return err
-       }*/
-       err := CreateDbProject(project)
-       if err != nil {
-               return err
+       clauses := []dal.Clause{
+               dal.From(&models.Project{}),
        }
-       return nil
-}
 
-// CreateProjectMetric accepts a ProjectMetric instance and insert it to 
database
-func CreateProjectMetric(projectMetric *models.ProjectMetric) errors.Error {
-       /*enProjectMetric, err := encryptProjectMetric(projectMetric)
+       count, err := db.Count(clauses...)
        if err != nil {
-               return err
-       }*/
-       err := CreateDbProjectMetric(projectMetric)
-       if err != nil {
-               return err
+               return nil, 0, errors.Default.Wrap(err, "error getting DB count 
of project")
        }
-       return nil
-}
 
-// GetProject returns a Project
-func GetProject(name string) (*models.Project, errors.Error) {
-       project, err := GetDbProject(name)
+       clauses = append(clauses,
+               dal.Orderby("created_at DESC"),
+               dal.Offset(query.GetSkip()),
+               dal.Limit(query.GetPageSize()),
+       )
+       projects := make([]*models.Project, 0)
+       err = db.All(&projects, clauses...)
        if err != nil {
-               return nil, errors.Convert(err)
+               return nil, 0, errors.Default.Wrap(err, "error finding DB 
project")
        }
 
-       /*project, err = decryptProject(project)
-       if err != nil {
-               return nil, errors.Convert(err)
-       }*/
-
-       return project, nil
+       return projects, count, nil
 }
 
-// GetProjectMetric returns a ProjectMetric
-func GetProjectMetric(projectName string, pluginName string) 
(*models.ProjectMetric, errors.Error) {
-       projectMetric, err := GetDbProjectMetric(projectName, pluginName)
-       if err != nil {
-               return nil, errors.Convert(err)
+// CreateProject accepts a project instance and insert it to database
+func CreateProject(projectInput *models.ApiInputProject) 
(*models.ApiOutputProject, errors.Error) {
+       // verify input
+       if err := VerifyStruct(projectInput); err != nil {
+               return nil, err
        }
 
-       /*projectMetric, err = decryptProjectMetric(projectMetric)
-       if err != nil {
-               return nil, errors.Convert(err)
-       }*/
-
-       return projectMetric, nil
-}
+       // create transaction to updte multiple tables
+       var err errors.Error
+       tx := db.Begin()
+       defer func() {
+               if r := recover(); r != nil || err != nil {
+                       err = tx.Rollback()
+                       if err != nil {
+                               log.Error(err, "PatchProject: failed to 
rollback")
+                       }
+               }
+       }()
 
-// FlushProjectMetrics remove all Project metrics by project name and create 
new metrics by baseMetrics
-func FlushProjectMetrics(projectName string, baseMetrics *[]models.BaseMetric) 
errors.Error {
-       err := removeAllDbProjectMetricsByProjectName(projectName)
+       // create project first
+       project := &models.Project{}
+       project.BaseProject = projectInput.BaseProject
+       err = db.Create(project)
        if err != nil {
-               return errors.Default.Wrap(err, fmt.Sprintf("error to 
removeAllDbProjectMetricsByProjectName for %s", projectName))
-       }
-
-       for _, baseMetric := range *baseMetrics {
-               err = CreateProjectMetric(&models.ProjectMetric{
-                       BaseProjectMetric: models.BaseProjectMetric{
-                               ProjectName: projectName,
-                               BaseMetric:  baseMetric,
-                       },
-               })
-               if err != nil {
-                       return errors.Default.Wrap(err, fmt.Sprintf("failed to  
CreateProjectMetric for [%s][%s]", projectName, baseMetric.PluginName))
+               if db.IsDuplicationError(err) {
+                       return nil, errors.BadInput.New(fmt.Sprintf("A project 
with name [%s] already exists", project.Name))
                }
+               return nil, errors.Default.Wrap(err, "error creating DB 
project")
        }
 
-       return nil
-}
-
-// LoadBluePrintAndMetrics load the blueprint and ProjectMetrics for 
projectOutputv
-func LoadBluePrintAndMetrics(projectOutput *models.ApiOutputProject) 
errors.Error {
-       var err errors.Error
-
-       // load Metrics
-       projectMetrics, count, err := GetProjectMetrics(projectOutput.Name)
-       if err != nil {
-               return errors.Default.Wrap(err, "Failed to get project metrics 
by project")
-       }
-       if count == 0 {
-               projectOutput.Metrics = nil
-       } else {
-               baseMetric := make([]models.BaseMetric, len(*projectMetrics))
-               for i, projectMetric := range *projectMetrics {
-                       baseMetric[i] = projectMetric.BaseMetric
+       // check if need flush the Metrics
+       if projectInput.Metrics != nil {
+               err = refreshProjectMetrics(tx, projectInput)
+               if err != nil {
+                       return nil, err
                }
-               projectOutput.Metrics = &baseMetric
        }
 
-       // load blueprint
-       projectOutput.Blueprint, err = 
GetBlueprintByProjectName(projectOutput.Name)
+       // all good, commit transaction
+       err = tx.Commit()
        if err != nil {
-               return errors.Default.Wrap(err, "Error to get blueprint by 
project")
+               return nil, err
        }
 
-       return nil
+       return makeProjectOutput(&projectInput.BaseProject)
 }
 
-// GetProjectMetrics returns all ProjectMetric of the project
-func GetProjectMetrics(projectName string) (*[]models.ProjectMetric, int64, 
errors.Error) {
-       projectMetrics, count, err := GetDbProjectMetrics(projectName)
-       if err != nil {
-               return nil, 0, errors.Convert(err)
+// GetProject returns a Project
+func GetProject(name string) (*models.ApiOutputProject, errors.Error) {
+       // verify input
+       if name == "" {
+               return nil, errors.BadInput.New("project name is missing")
        }
 
-       /*for i, projectMetric := range projectMetrics {
-               projectMetrics[i], err = decryptProjectMetric(projectMetric)
-               if err != nil {
-                       return nil, 0, err
-               }
-       }*/
-
-       return projectMetrics, count, nil
-}
-
-// GetProjects returns a paginated list of Projects based on `query`
-func GetProjects(query *ProjectQuery) ([]*models.Project, int64, errors.Error) 
{
-       projects, count, err := GetDbProjects(query)
+       // load project
+       project := &models.Project{}
+       err := db.First(project, dal.Where("name = ?", name))
        if err != nil {
-               return nil, 0, errors.Convert(err)
-       }
-
-       /*for i, project := range projects {
-               projects[i], err = decryptProject(project)
-               if err != nil {
-                       return nil, 0, err
+               if db.IsErrorNotFound(err) {
+                       return nil, errors.NotFound.Wrap(err, 
fmt.Sprintf("could not find project [%s] in DB", name))
                }
-       }*/
+               return nil, errors.Default.Wrap(err, "error getting project 
from DB")
+       }
 
-       return projects, count, nil
+       // convert to api output
+       return makeProjectOutput(&project.BaseProject)
 }
 
 // PatchProject FIXME ...
 func PatchProject(name string, body map[string]interface{}) 
(*models.ApiOutputProject, errors.Error) {
        projectInput := &models.ApiInputProject{}
-       projectOutput := &models.ApiOutputProject{}
 
-       // load record from db
-       project, err := GetProject(name)
+       // load input
+       err := helper.DecodeMapStruct(body, projectInput)
        if err != nil {
                return nil, err
        }
 
-       err = helper.DecodeMapStruct(body, projectInput)
+       // wrap all operation inside a transaction
+       tx := db.Begin()
+       defer func() {
+               if r := recover(); r != nil || err != nil {
+                       err = tx.Rollback()

Review Comment:
   And rollback may not be called if other err returned?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to