This is an automated email from the ASF dual-hosted git repository.

likyh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-devlake.git


The following commit(s) were added to refs/heads/main by this push:
     new b4ba6254c fix: remove createdDateAfter completely (#4507)
b4ba6254c is described below

commit b4ba6254c69826cc63da90ea05a4722b9d5a9af0
Author: Klesh Wong <[email protected]>
AuthorDate: Thu Feb 23 21:01:58 2023 +0800

    fix: remove createdDateAfter completely (#4507)
    
    * fix: remove createdDateAfter completely
    
    * fix: IsIncremental logic error
---
 backend/core/dal/dal.go                            |  7 +-
 backend/core/models/blueprint.go                   | 12 ++-
 backend/core/models/collector_state.go             | 10 +--
 ...remove_createddateafter_from_collector_state.go | 96 ++++++++++++++++++++++
 backend/core/models/migrationscripts/register.go   |  1 +
 backend/core/plugin/plugin_blueprint.go            |  8 +-
 .../pluginhelper/api/api_collector_with_state.go   | 49 ++++-------
 backend/impls/dalgorm/dalgorm.go                   | 15 ++--
 .../plugins/github/tasks/pr_review_collector.go    |  2 +-
 .../github/tasks/pr_review_comment_collector.go    |  2 +-
 backend/plugins/jira/jira.go                       |  8 +-
 backend/server/services/blueprint.go               |  4 +-
 backend/test/helper/api.go                         | 16 ++--
 13 files changed, 151 insertions(+), 79 deletions(-)

diff --git a/backend/core/dal/dal.go b/backend/core/dal/dal.go
index af2f93dff..b60ca980b 100644
--- a/backend/core/dal/dal.go
+++ b/backend/core/dal/dal.go
@@ -19,8 +19,9 @@ package dal
 
 import (
        "database/sql"
-       "github.com/apache/incubator-devlake/core/errors"
        "reflect"
+
+       "github.com/apache/incubator-devlake/core/errors"
 )
 
 type Tabler interface {
@@ -108,9 +109,9 @@ type Dal interface {
        // Update updates record
        Update(entity interface{}, clauses ...Clause) errors.Error
        // UpdateColumn allows you to update multiple records
-       UpdateColumn(entity interface{}, columnName string, value interface{}, 
clauses ...Clause) errors.Error
+       UpdateColumn(entityOrTable interface{}, columnName string, value 
interface{}, clauses ...Clause) errors.Error
        // UpdateColumns allows you to update multiple columns of multiple 
records
-       UpdateColumns(entity interface{}, set []DalSet, clauses ...Clause) 
errors.Error
+       UpdateColumns(entityOrTable interface{}, set []DalSet, clauses 
...Clause) errors.Error
        // UpdateAllColumn updated all Columns of entity
        UpdateAllColumn(entity interface{}, clauses ...Clause) errors.Error
        // CreateOrUpdate tries to create the record, or fallback to update all 
if failed
diff --git a/backend/core/models/blueprint.go b/backend/core/models/blueprint.go
index 5d1ebcdfe..dec9ac119 100644
--- a/backend/core/models/blueprint.go
+++ b/backend/core/models/blueprint.go
@@ -48,13 +48,11 @@ type Blueprint struct {
 }
 
 type BlueprintSettings struct {
-       Version string `json:"version" validate:"required,semver,oneof=1.0.0"`
-       // Deprecating(timeAfter): copy to TimeAfter and delete the field in 
last step
-       CreatedDateAfter *time.Time      `json:"createdDateAfter"`
-       TimeAfter        *time.Time      `json:"timeAfter"`
-       Connections      json.RawMessage `json:"connections" 
validate:"required"`
-       BeforePlan       json.RawMessage `json:"before_plan"`
-       AfterPlan        json.RawMessage `json:"after_plan"`
+       Version     string          `json:"version" 
validate:"required,semver,oneof=1.0.0"`
+       TimeAfter   *time.Time      `json:"timeAfter"`
+       Connections json.RawMessage `json:"connections" validate:"required"`
+       BeforePlan  json.RawMessage `json:"before_plan"`
+       AfterPlan   json.RawMessage `json:"after_plan"`
 }
 
 // UnmarshalPlan unmarshals Plan in JSON to strong-typed plugin.PipelinePlan
diff --git a/backend/core/models/collector_state.go 
b/backend/core/models/collector_state.go
index 654556779..db36d548c 100644
--- a/backend/core/models/collector_state.go
+++ b/backend/core/models/collector_state.go
@@ -22,12 +22,10 @@ import (
 )
 
 type CollectorLatestState struct {
-       CreatedAt     time.Time `json:"createdAt"`
-       UpdatedAt     time.Time `json:"updatedAt"`
-       RawDataParams string    
`gorm:"primaryKey;column:raw_data_params;type:varchar(255);index" 
json:"raw_data_params"`
-       RawDataTable  string    
`gorm:"primaryKey;column:raw_data_table;type:varchar(255)" 
json:"raw_data_table"`
-       // Deprecating(timeAfter): copy to TimeAfter and delete the field in 
last step
-       CreatedDateAfter   *time.Time
+       CreatedAt          time.Time `json:"createdAt"`
+       UpdatedAt          time.Time `json:"updatedAt"`
+       RawDataParams      string    
`gorm:"primaryKey;column:raw_data_params;type:varchar(255);index" 
json:"raw_data_params"`
+       RawDataTable       string    
`gorm:"primaryKey;column:raw_data_table;type:varchar(255)" 
json:"raw_data_table"`
        TimeAfter          *time.Time
        LatestSuccessStart *time.Time
 }
diff --git 
a/backend/core/models/migrationscripts/20230223_remove_createddateafter_from_collector_state.go
 
b/backend/core/models/migrationscripts/20230223_remove_createddateafter_from_collector_state.go
new file mode 100644
index 000000000..0570f43c2
--- /dev/null
+++ 
b/backend/core/models/migrationscripts/20230223_remove_createddateafter_from_collector_state.go
@@ -0,0 +1,96 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package migrationscripts
+
+import (
+       "encoding/json"
+       "fmt"
+
+       "github.com/apache/incubator-devlake/core/context"
+       "github.com/apache/incubator-devlake/core/dal"
+       "github.com/apache/incubator-devlake/core/errors"
+)
+
+type blueprint20230223 struct {
+       ID       int64
+       Settings json.RawMessage `gorm:"serializer:encdec"`
+}
+
+func (blueprint20230223) TableName() string {
+       return "_devlake_blueprints"
+}
+
+type removeCreatedDateAfterFromCollectorMeta20230223 struct{}
+
+func (script *removeCreatedDateAfterFromCollectorMeta20230223) Up(basicRes 
context.BasicRes) errors.Error {
+       db := basicRes.GetDal()
+       // step 1: rename bp.settings.createdDateAfter to timeAfter
+       bp := &blueprint20230223{}
+       cursor, err := db.Cursor(dal.From(bp), dal.Where("mode = ?", "NORMAL"))
+       if err != nil {
+               return err
+       }
+       defer cursor.Close()
+       for cursor.Next() {
+               err = db.Fetch(cursor, bp)
+               if err != nil {
+                       return err
+               }
+               settingsMap := make(map[string]interface{})
+               if e := json.Unmarshal(bp.Settings, &settingsMap); e != nil {
+                       return errors.Default.Wrap(e, fmt.Sprintf("failed to 
unmarshal settings for blueprint #%v", bp.ID))
+               }
+               if v, ok := settingsMap["createdDateAfter"]; ok {
+                       settingsMap["timeAfter"] = v
+                       delete(settingsMap, "createdDateAfter")
+               } else {
+                       continue
+               }
+               if s, e := json.Marshal(settingsMap); e == nil {
+                       bp.Settings = s
+                       err = db.Update(bp)
+                       if err != nil {
+                               return err
+                       }
+               } else {
+                       return errors.Default.Wrap(e, fmt.Sprintf("failed to 
update settings for blueprint #%v", bp.ID))
+               }
+       }
+
+       // step 2: update collector_latest_state.time_after with values from 
created_date_after
+       table := "_devlake_collector_latest_state"
+       err = db.UpdateColumn(
+               table,
+               "time_after", dal.Expr("created_date_after"),
+               dal.Where("time_after IS NULL"),
+       )
+       if err != nil {
+               return err
+       }
+
+       // step 3: drop collector_latest_state.created_date_after
+       return db.DropColumns(table, "created_date_after")
+}
+
+func (*removeCreatedDateAfterFromCollectorMeta20230223) Version() uint64 {
+       return 20230223200040
+}
+
+func (*removeCreatedDateAfterFromCollectorMeta20230223) Name() string {
+       return "remove created_date_after from _devlake_collector_latest_state"
+}
diff --git a/backend/core/models/migrationscripts/register.go 
b/backend/core/models/migrationscripts/register.go
index ca5cb8320..c779d44a7 100644
--- a/backend/core/models/migrationscripts/register.go
+++ b/backend/core/models/migrationscripts/register.go
@@ -73,5 +73,6 @@ func All() []plugin.MigrationScript {
                new(addCodeQuality),
                new(modifyIssueStorypointToFloat64),
                new(addCommitShaIndex),
+               new(removeCreatedDateAfterFromCollectorMeta20230223),
        }
 }
diff --git a/backend/core/plugin/plugin_blueprint.go 
b/backend/core/plugin/plugin_blueprint.go
index c842f909d..0c681829d 100644
--- a/backend/core/plugin/plugin_blueprint.go
+++ b/backend/core/plugin/plugin_blueprint.go
@@ -173,9 +173,7 @@ type CompositePluginBlueprintV200 interface {
 }
 
 type BlueprintSyncPolicy struct {
-       Version    string `json:"version" 
validate:"required,semver,oneof=1.0.0"`
-       SkipOnFail bool   `json:"skipOnFail"`
-       // Deprecating(timeAfter): use TimeAfter instead
-       CreatedDateAfter *time.Time `json:"createdDateAfter"`
-       TimeAfter        *time.Time `json:"timeAfter"`
+       Version    string     `json:"version" 
validate:"required,semver,oneof=1.0.0"`
+       SkipOnFail bool       `json:"skipOnFail"`
+       TimeAfter  *time.Time `json:"timeAfter"`
 }
diff --git a/backend/helpers/pluginhelper/api/api_collector_with_state.go 
b/backend/helpers/pluginhelper/api/api_collector_with_state.go
index c506468cb..e6f727284 100644
--- a/backend/helpers/pluginhelper/api/api_collector_with_state.go
+++ b/backend/helpers/pluginhelper/api/api_collector_with_state.go
@@ -35,16 +35,14 @@ type ApiCollectorStateManager struct {
        RawDataSubTaskArgs
        // *ApiCollector
        // *GraphqlCollector
-       subtasks    []plugin.SubTask
-       LatestState models.CollectorLatestState
-       // Deprecating(timeAfter): to be deleted
-       CreatedDateAfter *time.Time
-       TimeAfter        *time.Time
-       ExecuteStart     time.Time
+       subtasks     []plugin.SubTask
+       LatestState  models.CollectorLatestState
+       TimeAfter    *time.Time
+       ExecuteStart time.Time
 }
 
-// NewApiCollectorWithStateEx create a new ApiCollectorStateManager
-func NewApiCollectorWithStateEx(args RawDataSubTaskArgs, createdDateAfter 
*time.Time, timeAfter *time.Time) (*ApiCollectorStateManager, errors.Error) {
+// NewApiCollectorWithState create a new ApiCollectorStateManager
+func NewStatefulApiCollector(args RawDataSubTaskArgs, timeAfter *time.Time) 
(*ApiCollectorStateManager, errors.Error) {
        db := args.Ctx.GetDal()
 
        rawDataSubTask, err := NewRawDataSubTask(args)
@@ -66,37 +64,24 @@ func NewApiCollectorWithStateEx(args RawDataSubTaskArgs, 
createdDateAfter *time.
        return &ApiCollectorStateManager{
                RawDataSubTaskArgs: args,
                LatestState:        latestState,
-               // Deprecating(timeAfter): to be deleted
-               CreatedDateAfter: createdDateAfter,
-               TimeAfter:        timeAfter,
-               ExecuteStart:     time.Now(),
+               TimeAfter:          timeAfter,
+               ExecuteStart:       time.Now(),
        }, nil
 }
 
-// NewApiCollectorWithState create a new ApiCollectorStateManager
-// Deprecating(timeAfter): use NewStatefulApiCollector instead
-func NewApiCollectorWithState(args RawDataSubTaskArgs, createdDateAfter 
*time.Time) (*ApiCollectorStateManager, errors.Error) {
-       return NewApiCollectorWithStateEx(args, createdDateAfter, nil)
-}
-
-// NewApiCollectorWithState create a new ApiCollectorStateManager
-func NewStatefulApiCollector(args RawDataSubTaskArgs, timeAfter *time.Time) 
(*ApiCollectorStateManager, errors.Error) {
-       return NewApiCollectorWithStateEx(args, nil, timeAfter)
-}
-
 // IsIncremental indicates if the collector should operate in incremental mode
 func (m *ApiCollectorStateManager) IsIncremental() bool {
-       // the initial collection
-       if m.LatestState.LatestSuccessStart == nil {
+       prevSyncTime := m.LatestState.LatestSuccessStart
+       prevTimeAfter := m.LatestState.TimeAfter
+       currTimeAfter := m.TimeAfter
+
+       if prevSyncTime == nil {
                return false
        }
-       // prioritize TimeAfter parameter: collector should filter data by 
`updated_date`
-       if m.TimeAfter != nil {
-               return m.LatestState.TimeAfter == nil || 
!m.TimeAfter.Before(*m.LatestState.TimeAfter)
+       if currTimeAfter != nil {
+               return prevTimeAfter == nil || 
!currTimeAfter.Before(*prevTimeAfter)
        }
-       // Deprecating(timeAfter): to be removed
-       // fallback to CreatedDateAfter: collector should filter data by 
`created_date`
-       return m.LatestState.CreatedDateAfter == nil || m.CreatedDateAfter != 
nil && !m.CreatedDateAfter.Before(*m.LatestState.CreatedDateAfter)
+       return prevTimeAfter == nil
 }
 
 // InitCollector init the embedded collector
@@ -132,8 +117,6 @@ func (m *ApiCollectorStateManager) Execute() errors.Error {
 
        db := m.Ctx.GetDal()
        m.LatestState.LatestSuccessStart = &m.ExecuteStart
-       // Deprecating(timeAfter): to be deleted
-       m.LatestState.CreatedDateAfter = m.CreatedDateAfter
        m.LatestState.TimeAfter = m.TimeAfter
        return db.CreateOrUpdate(&m.LatestState)
 }
diff --git a/backend/impls/dalgorm/dalgorm.go b/backend/impls/dalgorm/dalgorm.go
index a4f122305..342c3730a 100644
--- a/backend/impls/dalgorm/dalgorm.go
+++ b/backend/impls/dalgorm/dalgorm.go
@@ -20,11 +20,12 @@ package dalgorm
 import (
        "database/sql"
        "fmt"
+       "reflect"
+       "strings"
+
        "github.com/apache/incubator-devlake/core/dal"
        "github.com/apache/incubator-devlake/core/errors"
        "github.com/apache/incubator-devlake/core/utils"
-       "reflect"
-       "strings"
 
        "gorm.io/gorm"
        "gorm.io/gorm/clause"
@@ -201,15 +202,16 @@ func (d *Dalgorm) Delete(entity interface{}, clauses 
...dal.Clause) errors.Error
 }
 
 // UpdateColumn allows you to update mulitple records
-func (d *Dalgorm) UpdateColumn(entity interface{}, columnName string, value 
interface{}, clauses ...dal.Clause) errors.Error {
+func (d *Dalgorm) UpdateColumn(entityOrTable interface{}, columnName string, 
value interface{}, clauses ...dal.Clause) errors.Error {
        if expr, ok := value.(dal.DalClause); ok {
                value = gorm.Expr(expr.Expr, transformParams(expr.Params)...)
        }
-       return errors.Convert(buildTx(d.db, 
clauses).Model(entity).Update(columnName, value).Error)
+       clauses = append(clauses, dal.From(entityOrTable))
+       return errors.Convert(buildTx(d.db, clauses).Update(columnName, 
value).Error)
 }
 
 // UpdateColumns allows you to update multiple columns of mulitple records
-func (d *Dalgorm) UpdateColumns(entity interface{}, set []dal.DalSet, clauses 
...dal.Clause) errors.Error {
+func (d *Dalgorm) UpdateColumns(entityOrTable interface{}, set []dal.DalSet, 
clauses ...dal.Clause) errors.Error {
        updatesSet := make(map[string]interface{})
 
        for _, s := range set {
@@ -219,7 +221,8 @@ func (d *Dalgorm) UpdateColumns(entity interface{}, set 
[]dal.DalSet, clauses ..
                updatesSet[s.ColumnName] = s.Value
        }
 
-       return errors.Convert(buildTx(d.db, 
clauses).Model(entity).Updates(updatesSet).Error)
+       clauses = append(clauses, dal.From(entityOrTable))
+       return errors.Convert(buildTx(d.db, clauses).Updates(updatesSet).Error)
 }
 
 // UpdateAllColumn updated all Columns of entity
diff --git a/backend/plugins/github/tasks/pr_review_collector.go 
b/backend/plugins/github/tasks/pr_review_collector.go
index 86d022930..e5928cf9f 100644
--- a/backend/plugins/github/tasks/pr_review_collector.go
+++ b/backend/plugins/github/tasks/pr_review_collector.go
@@ -47,7 +47,7 @@ func CollectApiPullRequestReviews(taskCtx 
plugin.SubTaskContext) errors.Error {
        db := taskCtx.GetDal()
        data := taskCtx.GetData().(*GithubTaskData)
 
-       collectorWithState, err := 
helper.NewApiCollectorWithState(helper.RawDataSubTaskArgs{
+       collectorWithState, err := 
helper.NewStatefulApiCollector(helper.RawDataSubTaskArgs{
                Ctx: taskCtx,
                Params: GithubApiParams{
                        ConnectionId: data.Options.ConnectionId,
diff --git a/backend/plugins/github/tasks/pr_review_comment_collector.go 
b/backend/plugins/github/tasks/pr_review_comment_collector.go
index fb69becfb..e19349761 100644
--- a/backend/plugins/github/tasks/pr_review_comment_collector.go
+++ b/backend/plugins/github/tasks/pr_review_comment_collector.go
@@ -35,7 +35,7 @@ const RAW_PR_REVIEW_COMMENTS_TABLE = 
"github_api_pull_request_review_comments"
 func CollectPrReviewComments(taskCtx plugin.SubTaskContext) errors.Error {
        data := taskCtx.GetData().(*GithubTaskData)
 
-       collectorWithState, err := 
helper.NewApiCollectorWithState(helper.RawDataSubTaskArgs{
+       collectorWithState, err := 
helper.NewStatefulApiCollector(helper.RawDataSubTaskArgs{
                Ctx: taskCtx,
                Params: GithubApiParams{
                        ConnectionId: data.Options.ConnectionId,
diff --git a/backend/plugins/jira/jira.go b/backend/plugins/jira/jira.go
index 3e6569290..208b4c82e 100644
--- a/backend/plugins/jira/jira.go
+++ b/backend/plugins/jira/jira.go
@@ -32,12 +32,12 @@ func main() {
        boardId := cmd.Flags().Uint64P("board", "b", 0, "jira board id")
        _ = cmd.MarkFlagRequired("connection")
        _ = cmd.MarkFlagRequired("board")
-       CreatedDateAfter := cmd.Flags().StringP("createdDateAfter", "a", "", 
"collect data that are created after specified time, ie 2006-05-06T07:08:09Z")
+       timeAfter := cmd.Flags().StringP("timeAfter", "a", "", "collect data 
that are created after specified time, ie 2006-05-06T07:08:09Z")
        cmd.Run = func(c *cobra.Command, args []string) {
                runner.DirectRun(c, args, PluginEntry, map[string]interface{}{
-                       "connectionId":     *connectionId,
-                       "boardId":          *boardId,
-                       "createdDateAfter": *CreatedDateAfter,
+                       "connectionId": *connectionId,
+                       "boardId":      *boardId,
+                       "timeAfter":    *timeAfter,
                })
        }
        runner.RunCmd(cmd)
diff --git a/backend/server/services/blueprint.go 
b/backend/server/services/blueprint.go
index 158921bd9..8bba964db 100644
--- a/backend/server/services/blueprint.go
+++ b/backend/server/services/blueprint.go
@@ -274,14 +274,12 @@ func MakePlanForBlueprint(blueprint *models.Blueprint) 
(plugin.PipelinePlan, err
        }
 
        bpSyncPolicy := plugin.BlueprintSyncPolicy{}
-       // Deprecating(timeAfter): to be deleted
-       bpSyncPolicy.CreatedDateAfter = bpSettings.CreatedDateAfter
        bpSyncPolicy.TimeAfter = bpSettings.TimeAfter
 
        var plan plugin.PipelinePlan
        switch bpSettings.Version {
        case "1.0.0":
-               // Notice: v1 not complete SkipOnFail & CreatedDateAfter
+               // Notice: v1 not complete SkipOnFail & TimeAfter
                plan, err = GeneratePlanJsonV100(bpSettings)
        case "2.0.0":
                // load project metric plugins and convert it to a map
diff --git a/backend/test/helper/api.go b/backend/test/helper/api.go
index f22a9e70a..faf233bb1 100644
--- a/backend/test/helper/api.go
+++ b/backend/test/helper/api.go
@@ -65,21 +65,17 @@ func (d *DevlakeClient) ListConnections(pluginName string) 
[]*Connection {
 }
 
 type BlueprintV2Config struct {
-       Connection *plugin.BlueprintConnectionV200
-       // Deprecating(timeAfter): to be deleted
-       CreatedDateAfter *time.Time
-       TimeAfter        *time.Time
-       SkipOnFail       bool
-       ProjectName      string
+       Connection  *plugin.BlueprintConnectionV200
+       TimeAfter   *time.Time
+       SkipOnFail  bool
+       ProjectName string
 }
 
 // CreateBasicBlueprintV2 FIXME
 func (d *DevlakeClient) CreateBasicBlueprintV2(name string, config 
*BlueprintV2Config) models.Blueprint {
        settings := &models.BlueprintSettings{
-               Version: "2.0.0",
-               // Deprecating(timeAfter): to be deleted
-               CreatedDateAfter: config.CreatedDateAfter,
-               TimeAfter:        config.TimeAfter,
+               Version:   "2.0.0",
+               TimeAfter: config.TimeAfter,
                Connections: ToJson([]*plugin.BlueprintConnectionV200{
                        config.Connection,
                }),

Reply via email to