From ed2b3e156578de2425b7d0b928f2909ffe27997c Mon Sep 17 00:00:00 2001 From: Scott Lobdell Date: Mon, 30 Apr 2018 12:47:43 -0700 Subject: [PATCH] Bugfix for unicode characters in commit message Test: Regression tests included Change-Id: I92d4bc0a2962d8dd9d9a3c6fde53d7fbd37891e2 --- .../repodiff/controllers/differential.go | 4 +-- .../repodiff/controllers/regression_test.go | 31 +++++++++++++++++++ .../repodiff/controllers/testdata/commit.csv | 2 ++ .../repodiff/interactors/manifest_test.go | 10 +++--- .../service/repodiff/interactors/strings.go | 28 ++++++++++++++++- .../repodiff/interactors/strings_test.go | 20 ++++++++++++ .../service/repodiff/mappers/mappers.go | 3 +- 7 files changed, 89 insertions(+), 9 deletions(-) create mode 100644 tools/repo_diff/service/repodiff/controllers/regression_test.go create mode 100644 tools/repo_diff/service/repodiff/controllers/testdata/commit.csv diff --git a/tools/repo_diff/service/repodiff/controllers/differential.go b/tools/repo_diff/service/repodiff/controllers/differential.go index 3a4d772ba..62d8f1071 100644 --- a/tools/repo_diff/service/repodiff/controllers/differential.go +++ b/tools/repo_diff/service/repodiff/controllers/differential.go @@ -199,7 +199,7 @@ func readCSVFiles(projectCSVFile, commitCSVFile string) ([]ent.DiffRow, []ent.Co if err != nil { return nil, nil, errors.Wrap(err, "Error converting CSV file to entities") } - commitRows, err := csvFileToCommitRows(commitCSVFile) + commitRows, err := CSVFileToCommitRows(commitCSVFile) if err != nil { return nil, nil, errors.Wrap(err, "Error converting CSV file to entities") } @@ -252,7 +252,7 @@ func toDiffRows(entities []interface{}) ([]ent.DiffRow, error) { return diffRows, nil } -func csvFileToCommitRows(csvFile string) ([]ent.CommitRow, error) { +func CSVFileToCommitRows(csvFile string) ([]ent.CommitRow, error) { entities, err := filesystem.CSVFileToEntities( csvFile, func(cols []string) (interface{}, error) { diff --git a/tools/repo_diff/service/repodiff/controllers/regression_test.go b/tools/repo_diff/service/repodiff/controllers/regression_test.go new file mode 100644 index 000000000..57c3de285 --- /dev/null +++ b/tools/repo_diff/service/repodiff/controllers/regression_test.go @@ -0,0 +1,31 @@ +package controllers + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + cst "repodiff/constants" + ent "repodiff/entities" + "repodiff/repositories" +) + +func TestRegressionIncorrectStringValue(t *testing.T) { + commitRows, _ := CSVFileToCommitRows("testdata/commit.csv") + analyzed := make([]ent.AnalyzedCommitRow, len(commitRows)) + for i, row := range commitRows { + analyzed[i] = ent.AnalyzedCommitRow{ + CommitRow: row, + Type: cst.Empty, + } + } + + c, _ := repositories.NewCommitRepository( + ent.MappedDiffTarget{ + UpstreamTarget: 1, + DownstreamTarget: 2, + }, + ) + err := c.InsertCommitRows(analyzed) + assert.Equal(t, nil, err, "Error should be nil") +} diff --git a/tools/repo_diff/service/repodiff/controllers/testdata/commit.csv b/tools/repo_diff/service/repodiff/controllers/testdata/commit.csv new file mode 100644 index 000000000..579eb6d95 --- /dev/null +++ b/tools/repo_diff/service/repodiff/controllers/testdata/commit.csv @@ -0,0 +1,2 @@ +Date,Commit,Downstream Project,Author,Subject +2018/04/28,f3bc9021add1f9cb458223dd374c58c69f53e207,platform/frameworks/support,aurimas@google.com,Move to AGP 3.0.0 stable 😁 diff --git a/tools/repo_diff/service/repodiff/interactors/manifest_test.go b/tools/repo_diff/service/repodiff/interactors/manifest_test.go index facb43455..166b34926 100644 --- a/tools/repo_diff/service/repodiff/interactors/manifest_test.go +++ b/tools/repo_diff/service/repodiff/interactors/manifest_test.go @@ -5,19 +5,19 @@ import ( "github.com/stretchr/testify/assert" - c "repodiff/constants" - e "repodiff/entities" + cst "repodiff/constants" + ent "repodiff/entities" "repodiff/persistence/filesystem" ) func TestProjectNamesToType(t *testing.T) { - var common, downstream, upstream e.ManifestFile + var common, downstream, upstream ent.ManifestFile filesystem.ReadXMLAsEntity("testdata/common_manifest.xml", &common) filesystem.ReadXMLAsEntity("testdata/downstream_manifest.xml", &downstream) filesystem.ReadXMLAsEntity("testdata/upstream_manifest.xml", &upstream) nameToType := ProjectNamesToType( - e.ManifestFileGroup{ + &ent.ManifestFileGroup{ Common: common, Upstream: upstream, Downstream: downstream, @@ -27,7 +27,7 @@ func TestProjectNamesToType(t *testing.T) { distinctCount := 0 for _, projectType := range nameToType { - if projectType == c.DifferentialSpecific { + if projectType == cst.DifferentialSpecific { distinctCount++ } } diff --git a/tools/repo_diff/service/repodiff/interactors/strings.go b/tools/repo_diff/service/repodiff/interactors/strings.go index 9f3c0ba3d..b28988e66 100644 --- a/tools/repo_diff/service/repodiff/interactors/strings.go +++ b/tools/repo_diff/service/repodiff/interactors/strings.go @@ -1,11 +1,15 @@ package interactors import ( + "regexp" "sort" + "strings" ) type simpleSet map[string]bool +var unicode = regexp.MustCompile("[^\x00-\x7F]+") + func (s simpleSet) Contains(other string) bool { enabled, exists := s[other] return exists && enabled @@ -45,7 +49,7 @@ func SetSubtract(add, negate []string) []string { } func SetUnion(slice1, slice2 []string) []string { - return allKeys( + union := allKeys( sliceToSimpleSet( append( slice1, @@ -53,6 +57,8 @@ func SetUnion(slice1, slice2 []string) []string { ), ), ) + sort.Strings(union) + return union } func sliceToSimpleSet(s []string) simpleSet { @@ -87,3 +93,23 @@ func allKeys(sets ...simpleSet) []string { } return keys } + +func FilterNoUnicode(s string) string { + badCharacters := sliceToSimpleSet( + unicode.FindAllString(s, -1), + ) + if len(badCharacters) == 0 { + return s + } + validCharacters := make([]string, 0, len(s)) + for _, rune_ := range s { + char := string(rune_) + if !badCharacters.Contains(char) { + validCharacters = append(validCharacters, char) + } + } + return strings.Join( + validCharacters, + "", + ) +} diff --git a/tools/repo_diff/service/repodiff/interactors/strings_test.go b/tools/repo_diff/service/repodiff/interactors/strings_test.go index 6ae1dcd97..6845198bf 100644 --- a/tools/repo_diff/service/repodiff/interactors/strings_test.go +++ b/tools/repo_diff/service/repodiff/interactors/strings_test.go @@ -90,3 +90,23 @@ func TestSetUnion(t *testing.T) { union := SetUnion(s1, s2) assert.Equal(t, expected, union, "Union of s2 and s1") } + +func TestFilterNoUnicodeWithUnicode(t *testing.T) { + regressionStr := "Move to AGP 3.0.0 stable 😁" + assert.Equal( + t, + "Move to AGP 3.0.0 stable ", + FilterNoUnicode(regressionStr), + "Function should filter out unicode characters", + ) +} + +func TestFilterNoUnicodeWithNoUnicode(t *testing.T) { + validStr := "I'm a regular string with no whacky unicode chars" + assert.Equal( + t, + validStr, + FilterNoUnicode(validStr), + "No change should occur", + ) +} diff --git a/tools/repo_diff/service/repodiff/mappers/mappers.go b/tools/repo_diff/service/repodiff/mappers/mappers.go index 72f5d693f..cf2612e2f 100644 --- a/tools/repo_diff/service/repodiff/mappers/mappers.go +++ b/tools/repo_diff/service/repodiff/mappers/mappers.go @@ -12,6 +12,7 @@ import ( "repodiff/constants" e "repodiff/entities" + "repodiff/interactors" "repodiff/utils" ) @@ -123,7 +124,7 @@ func commitRowToPersistCols(c e.AnalyzedCommitRow, uuidBytes string, timestamp i c.Commit, c.DownstreamProject, c.Author, - c.Subject, + interactors.FilterNoUnicode(c.Subject), c.Type, } }