You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by li...@apache.org on 2023/02/23 13:02:03 UTC

[incubator-devlake] branch main updated: fix: remove createdDateAfter completely (#4507)

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 <zh...@merico.dev>
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,
 		}),