Repository: dragonflyoss/Dragonfly2
Pull request #211 (2021-05-10 08:47:30)

Refactor idgen pkg
<!--- Provide a general summary of your changes in the Title above -->

## Description
- Generate sha256 from slice.
- Standardized naming.

## Related Issue
#210 
<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps to reproduce -->
<!--- Please link to the issue here: -->

## Motivation and Context
N/A
<!--- Why is this change required? What problem does it solve? -->

## Screenshots (if appropriate):

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Documentation Update (if none of the other choices apply)

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [ ] Code compiles correctly.
- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have read the **CONTRIBUTING** document.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.



Patch:
diff --git a/pkg/idgen/task_id.go b/pkg/idgen/task_id.go
index b9d2f9cf..878e0246 100644
--- a/pkg/idgen/task_id.go
+++ b/pkg/idgen/task_id.go
@@ -17,56 +17,51 @@
 package idgen
 
 import (
+	"hash/crc32"
 	"strings"
 
 	"d7y.io/dragonfly/v2/pkg/rpc/base"
 	"d7y.io/dragonfly/v2/pkg/util/digestutils"
 	"d7y.io/dragonfly/v2/pkg/util/net/urlutils"
-	"d7y.io/dragonfly/v2/pkg/util/stringutils"
 )
 
 const (
-	TwinsA = "_A"
-	TwinsB = "_B"
+	TwinsASuffix = "_A"
+	TwinsBSuffix = "_B"
 )
 
 // GenerateTaskID generates a taskId.
 // filter is separated by & character.
-func GenerateTaskID(url string, filter string, meta *base.UrlMeta, bizID string) string {
-	taskIdSource := url
-	if filter != "" {
-		taskIdSource = urlutils.FilterURLParam(url, strings.Split(filter, "&"))
-	}
+func TaskID(url string, filter string, meta *base.UrlMeta, bizID string) string {
+	var data []string
+
+	data = append(data, urlutils.FilterURLParam(url, strings.Split(filter, "&")))
 
-	var md5String, rangeString string
 	if meta != nil {
-		md5String = meta.Md5
-		rangeString = meta.Range
-	}
+		if meta.Md5 != "" {
+			data = append(data, meta.Md5)
+		}
 
-	if md5String != "" {
-		taskIdSource += "|" + md5String
-	} else if bizID != "" {
-		taskIdSource += "|" + bizID
+		if meta.Range != "" {
+			data = append(data, meta.Range)
+		}
 	}
 
-	if rangeString != "" {
-		taskIdSource += "|" + rangeString
+	if bizID != "" {
+		data = append(data, bizID)
 	}
 
-	taskIdSource = stringutils.SubString(taskIdSource, len(taskIdSource)-10, len(taskIdSource)) + taskIdSource
-
-	return digestutils.Sha256(taskIdSource)
+	return digestutils.Sha256(data...)
 }
 
 // GenerateTwinsTaskId used A/B testing
-func GenerateTwinsTaskID(url string, filter string, meta *base.UrlMeta, bizID, peerID string) string {
-	taskID := GenerateTaskID(url, filter, meta, bizID)
+func TwinsTaskID(url string, filter string, meta *base.UrlMeta, bizID, peerID string) string {
+	taskID := TaskID(url, filter, meta, bizID)
 
-	if digestutils.CheckSum(peerID)&1 == 0 {
-		taskID += TwinsA
+	if crc32.ChecksumIEEE([]byte(peerID))&1 == 0 {
+		taskID += TwinsASuffix
 	} else {
-		taskID += TwinsB
+		taskID += TwinsBSuffix
 	}
 
 	return taskID
diff --git a/pkg/util/digestutils/digest.go b/pkg/util/digestutils/digest.go
index cb4baeb5..4a8119c3 100644
--- a/pkg/util/digestutils/digest.go
+++ b/pkg/util/digestutils/digest.go
@@ -22,7 +22,6 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"hash"
-	"hash/crc32"
 	"io"
 
 	"d7y.io/dragonfly/v2/pkg/unit"
@@ -44,10 +43,6 @@ func Sha256(values ...string) string {
 	return ToHashString(h)
 }
 
-func CheckSum(value string) uint32 {
-	return crc32.ChecksumIEEE([]byte(value))
-}
-
 func Md5Reader(reader io.Reader) string {
 	h := md5.New()
 	if _, err := io.Copy(h, reader); err != nil {
diff --git a/scheduler/scheduler/evaluator_factory.go b/scheduler/scheduler/evaluator_factory.go
index 5f094d11..c5399585 100644
--- a/scheduler/scheduler/evaluator_factory.go
+++ b/scheduler/scheduler/evaluator_factory.go
@@ -68,7 +68,7 @@ func (ef *evaluatorFactory) get(task *types.Task) Evaluator {
 
 	if ef.abtest {
 		name := ""
-		if strings.HasSuffix(task.TaskId, idgen.TwinsB) {
+		if strings.HasSuffix(task.TaskId, idgen.TwinsBSuffix) {
 			if ef.bscheduler != "" {
 				name = ef.bscheduler
 			}
diff --git a/scheduler/service/service.go b/scheduler/service/service.go
index 29a2a13b..ccca24d8 100644
--- a/scheduler/service/service.go
+++ b/scheduler/service/service.go
@@ -49,9 +49,9 @@ func NewSchedulerService(cfg *config.Config) *SchedulerService {
 
 func (s *SchedulerService) GenerateTaskID(url string, filter string, meta *base.UrlMeta, bizID string, peerID string) (taskID string) {
 	if s.ABTest {
-		return idgen.GenerateTwinsTaskID(url, filter, meta, bizID, peerID)
+		return idgen.TwinsTaskID(url, filter, meta, bizID, peerID)
 	}
-	return idgen.GenerateTaskID(url, filter, meta, bizID)
+	return idgen.TaskID(url, filter, meta, bizID)
 }
 
 func (s *SchedulerService) GetTask(taskID string) (task *types.Task, err error) {


Test patch:
diff --git a/pkg/dynconfig/dynconfig_test.go b/pkg/dynconfig/dynconfig_test.go
index b66bc060..566f90e3 100644
--- a/pkg/dynconfig/dynconfig_test.go
+++ b/pkg/dynconfig/dynconfig_test.go
@@ -46,7 +46,7 @@ func TestDynconfigGet_ManagerSourceType(t *testing.T) {
 		sleep          func()
 		cleanFileCache func(t *testing.T)
 		mock           func(m *mock_manager_client.MockmanagerClientMockRecorder)
-		expect         func(t *testing.T, res interface{})
+		expect         func(t *testing.T, data interface{})
 	}{
 		{
 			name:   "get dynconfig success without file cache",
@@ -196,7 +196,7 @@ func TestDynconfigUnmarshal_ManagerSourceType(t *testing.T) {
 		sleep          func()
 		cleanFileCache func(t *testing.T)
 		mock           func(m *mock_manager_client.MockmanagerClientMockRecorder)
-		expect         func(t *testing.T, res interface{})
+		expect         func(t *testing.T, data interface{})
 	}{
 		{
 			name:   "unmarshals dynconfig success without file cache",
@@ -343,7 +343,7 @@ func TestDynconfigGet_LocalSourceType(t *testing.T) {
 		dynconfig  TestDynconfig
 		configPath string
 		sleep      func()
-		expect     func(t *testing.T, res interface{})
+		expect     func(t *testing.T, data interface{})
 	}{
 		{
 			name:   "get dynconfig success with local file",
@@ -419,7 +419,7 @@ func TestDynconfigUnmarshal_LocalSourceType(t *testing.T) {
 		dynconfig  TestDynconfig
 		configPath string
 		sleep      func()
-		expect     func(t *testing.T, res interface{})
+		expect     func(t *testing.T, data interface{})
 	}{
 		{
 			name:   "unmarshals dynconfig success with local file",
diff --git a/pkg/idgen/task_id_test.go b/pkg/idgen/task_id_test.go
index 3be77987..85576d9d 100644
--- a/pkg/idgen/task_id_test.go
+++ b/pkg/idgen/task_id_test.go
@@ -19,15 +19,155 @@ package idgen
 import (
 	"testing"
 
+	"d7y.io/dragonfly/v2/pkg/rpc/base"
 	"github.com/stretchr/testify/assert"
 )
 
-func TestGenerateTaskId(t *testing.T) {
-	taskId := GenerateTaskId("http://alibaba.com/path/xx", "", nil, "")
-	assert.NotEmpty(t, taskId)
+func TestTaskID(t *testing.T) {
+	tests := []struct {
+		name   string
+		url    string
+		filter string
+		meta   *base.UrlMeta
+		bizID  string
+		expect func(t *testing.T, d interface{})
+	}{
+		{
+			name:   "generate taskID with url",
+			url:    "https://example.com",
+			filter: "",
+			meta:   nil,
+			bizID:  "",
+			expect: func(t *testing.T, d interface{}) {
+				assert := assert.New(t)
+				assert.Equal("100680ad546ce6a577f42f52df33b4cfdca756859e664b8d7de329b150d09ce9", d)
+			},
+		},
+		{
+			name:   "generate taskID with meta",
+			url:    "https://example.com",
+			filter: "",
+			meta: &base.UrlMeta{
+				Range: "foo",
+				Md5:   "bar",
+			},
+			bizID: "",
+			expect: func(t *testing.T, d interface{}) {
+				assert := assert.New(t)
+				assert.Equal("aeee0e0a2a0c75130582641353c539aaf9011a0088b31347f7588e70e449a3e0", d)
+			},
+		},
+		{
+			name:   "generate taskID with filter",
+			url:    "https://example.com?foo=foo&bar=bar",
+			filter: "foo&bar",
+			meta:   nil,
+			bizID:  "",
+			expect: func(t *testing.T, d interface{}) {
+				assert := assert.New(t)
+				assert.Equal("100680ad546ce6a577f42f52df33b4cfdca756859e664b8d7de329b150d09ce9", d)
+			},
+		},
+		{
+			name:   "generate taskID with bizID",
+			url:    "https://example.com",
+			filter: "",
+			meta:   nil,
+			bizID:  "foo",
+			expect: func(t *testing.T, d interface{}) {
+				assert := assert.New(t)
+				assert.Equal("2773851c628744fb7933003195db436ce397c1722920696c4274ff804d86920b", d)
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			data := TaskID(tc.url, tc.filter, tc.meta, tc.bizID)
+			tc.expect(t, data)
+		})
+	}
 }
 
-func TestGenerateTwinsTaskId(t *testing.T) {
-	taskId := GenerateTwinsTaskId("http://alibaba.com/path/xx", "", nil, "", "peerId")
-	assert.NotEmpty(t, taskId)
+func TestTwinsTaskID(t *testing.T) {
+	tests := []struct {
+		name   string
+		url    string
+		filter string
+		meta   *base.UrlMeta
+		bizID  string
+		peerID string
+		expect func(t *testing.T, d interface{})
+	}{
+		{
+			name:   "generate taskID with url",
+			url:    "https://example.com",
+			filter: "",
+			meta:   nil,
+			bizID:  "",
+			peerID: "foo",
+			expect: func(t *testing.T, d interface{}) {
+				assert := assert.New(t)
+				assert.Equal("100680ad546ce6a577f42f52df33b4cfdca756859e664b8d7de329b150d09ce9_B", d)
+			},
+		},
+		{
+			name:   "generate taskID with meta",
+			url:    "https://example.com",
+			filter: "",
+			meta: &base.UrlMeta{
+				Range: "foo",
+				Md5:   "bar",
+			},
+			bizID:  "",
+			peerID: "foo",
+			expect: func(t *testing.T, d interface{}) {
+				assert := assert.New(t)
+				assert.Equal("aeee0e0a2a0c75130582641353c539aaf9011a0088b31347f7588e70e449a3e0_B", d)
+			},
+		},
+		{
+			name:   "generate taskID with filter",
+			url:    "https://example.com?foo=foo&bar=bar",
+			filter: "foo&bar",
+			meta:   nil,
+			bizID:  "",
+			peerID: "foo",
+			expect: func(t *testing.T, d interface{}) {
+				assert := assert.New(t)
+				assert.Equal("100680ad546ce6a577f42f52df33b4cfdca756859e664b8d7de329b150d09ce9_B", d)
+			},
+		},
+		{
+			name:   "generate taskID with bizID",
+			url:    "https://example.com",
+			filter: "",
+			meta:   nil,
+			bizID:  "foo",
+			peerID: "foo",
+			expect: func(t *testing.T, d interface{}) {
+				assert := assert.New(t)
+				assert.Equal("2773851c628744fb7933003195db436ce397c1722920696c4274ff804d86920b_B", d)
+			},
+		},
+		{
+			name:   "generate twinsA taskID",
+			url:    "https://example.com",
+			filter: "",
+			meta:   nil,
+			bizID:  "",
+			peerID: "bar",
+			expect: func(t *testing.T, d interface{}) {
+				assert := assert.New(t)
+				assert.Equal("100680ad546ce6a577f42f52df33b4cfdca756859e664b8d7de329b150d09ce9_A", d)
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			data := TwinsTaskID(tc.url, tc.filter, tc.meta, tc.bizID, tc.peerID)
+			tc.expect(t, data)
+		})
+	}
 }