From a06d8571409eed1e9779cdf29e2baa444d296662 Mon Sep 17 00:00:00 2001 From: Katsuhiko YOSHIDA Date: Thu, 26 Oct 2023 19:31:26 +0900 Subject: [PATCH] Refactor names --- cmd/bundler.go | 58 +++++++++++------------ cmd/bundler_test.go | 46 ++++++++++++++++++ cmd/diagnose.go | 36 +++++++------- cmd/diagnose_test.go | 2 +- cmd/ruby/bundler/bundler.go | 94 ------------------------------------- cmd/yarn.go | 50 ++++++++++++-------- cmd/yarn_test.go | 17 ++----- 7 files changed, 131 insertions(+), 172 deletions(-) create mode 100644 cmd/bundler_test.go delete mode 100644 cmd/ruby/bundler/bundler.go diff --git a/cmd/bundler.go b/cmd/bundler.go index 096561a..c89479a 100644 --- a/cmd/bundler.go +++ b/cmd/bundler.go @@ -23,39 +23,17 @@ type GemResponse struct { HomepageUri string `json:"homepage_uri"` } -func FetchFromRubyGems(name string) (string, error) { - url := fmt.Sprintf(RUBYGEMS_ORG_API, name) - req, _ := http.NewRequest(http.MethodGet, url, nil) - client := new(http.Client) - resp, _ := client.Do(req) - body, _ := io.ReadAll(resp.Body) - - var Gem GemResponse - err := json.Unmarshal(body, &Gem) - if err != nil { - return "", errors.New("error: Unknown response") - } - - if Gem.SourceCodeUri != "" { - return Gem.SourceCodeUri, nil - } else if Gem.HomepageUri != "" { - return Gem.HomepageUri, nil - } - - return "", nil -} - -type BundlerStrategy struct { +type BundlerDoctor struct { } -func NewBundlerStrategy() *BundlerStrategy { - return &BundlerStrategy{} +func NewBundlerDoctor() *BundlerDoctor { + return &BundlerDoctor{} } -func (s *BundlerStrategy) Diagnose(r parser_io.ReadSeekerAt, year int) map[string]Diagnosis { +func (b *BundlerDoctor) Diagnose(r parser_io.ReadSeekerAt, year int) map[string]Diagnosis { diagnoses := make(map[string]Diagnosis) slicedNameWithOwners := [][]github.NameWithOwner{} - nameWithOwners := s.getNameWithOwners(r) + nameWithOwners := b.NameWithOwners(r) sliceSize := len(nameWithOwners) for i := 0; i < sliceSize; i += GITHUB_SEARCH_REPO_COUNT_PER_ONCE { @@ -94,7 +72,29 @@ func (s *BundlerStrategy) Diagnose(r parser_io.ReadSeekerAt, year int) map[strin return diagnoses } -func (s *BundlerStrategy) getNameWithOwners(r parser_io.ReadSeekerAt) []github.NameWithOwner { +func (d *BundlerDoctor) fetchURLFromRepository(name string) (string, error) { + url := fmt.Sprintf(RUBYGEMS_ORG_API, name) + req, _ := http.NewRequest(http.MethodGet, url, nil) + client := new(http.Client) + resp, _ := client.Do(req) + body, _ := io.ReadAll(resp.Body) + + var Gem GemResponse + err := json.Unmarshal(body, &Gem) + if err != nil { + return "", errors.New("error: Unknown response") + } + + if Gem.SourceCodeUri != "" { + return Gem.SourceCodeUri, nil + } else if Gem.HomepageUri != "" { + return Gem.HomepageUri, nil + } + + return "", nil +} + +func (d *BundlerDoctor) NameWithOwners(r parser_io.ReadSeekerAt) []github.NameWithOwner { var nameWithOwners []github.NameWithOwner p := &bundler.Parser{} libs, _, _ := p.Parse(r) @@ -102,7 +102,7 @@ func (s *BundlerStrategy) getNameWithOwners(r parser_io.ReadSeekerAt) []github.N for _, lib := range libs { fmt.Printf("%s\n", lib.Name) - githubUrl, err := FetchFromRubyGems(lib.Name) + githubUrl, err := d.fetchURLFromRepository(lib.Name) if err != nil { continue } diff --git a/cmd/bundler_test.go b/cmd/bundler_test.go new file mode 100644 index 0000000..8ea7d5e --- /dev/null +++ b/cmd/bundler_test.go @@ -0,0 +1,46 @@ +package cmd + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBundlerDoctor_fetchURLFromRepository(t *testing.T) { + tests := []struct { + name string + gem_name string + }{ + { + name: "source_code_uri exists", + gem_name: "rails", + }, + { + name: "no source_code_uri, but homepage_uri exists", + gem_name: "minitest", + }, + } + expects := []struct { + name string + url string + }{ + { + name: "source_code_uri exists", + url: "https://github.com/rails/rails", + }, + { + name: "no source_code_uri, but homepage_uri exists", + url: "https://github.com/minitest/minitest", + }, + } + + for i, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := BundlerDoctor{} + r, _ := s.fetchURLFromRepository(tt.gem_name) + expect := expects[i] + assert.Equal(t, true, strings.HasPrefix(r, expect.url)) + }) + } +} diff --git a/cmd/diagnose.go b/cmd/diagnose.go index 3ac30dd..28a4049 100644 --- a/cmd/diagnose.go +++ b/cmd/diagnose.go @@ -9,14 +9,18 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/aquasecurity/go-dep-parser/pkg/io" + parser_io "github.com/aquasecurity/go-dep-parser/pkg/io" "github.com/fatih/color" + "github.com/kyoshidajp/dep-doctor/cmd/github" "github.com/spf13/cobra" ) const MAX_YEAR_TO_BE_BLANK = 5 -type DiagnoseStrategy interface { +type Doctor interface { Diagnose(r io.ReadSeekerAt, year int) map[string]Diagnosis + fetchURLFromRepository(name string) (string, error) + NameWithOwners(r parser_io.ReadSeekerAt) []github.NameWithOwner } type Diagnosis struct { @@ -27,23 +31,18 @@ type Diagnosis struct { IsActive bool } -type Doctor struct { - strategy DiagnoseStrategy +type Department struct { + doctor Doctor } -func NewDoctor(strategy DiagnoseStrategy) *Doctor { - return &Doctor{ - strategy: strategy, +func NewDepartment(d Doctor) *Department { + return &Department{ + doctor: d, } } -func (d *Doctor) Diagnose(r io.ReadSeekCloserAt, year int) map[string]Diagnosis { - return d.strategy.Diagnose(r, year) -} - -var DoctorWithStrategy = map[string]DiagnoseStrategy{ - "bundler": NewBundlerStrategy(), - "yarn": NewYarnStrategy(), +func (d *Department) Diagnose(r io.ReadSeekCloserAt, year int) map[string]Diagnosis { + return d.doctor.Diagnose(r, year) } type Options struct { @@ -55,6 +54,11 @@ var ( o = &Options{} ) +var doctors = map[string]Doctor{ + "bundler": NewBundlerDoctor(), + "yarn": NewYarnDoctor(), +} + var diagnoseCmd = &cobra.Command{ Use: "diagnose", Short: "Diagnose dependencies", @@ -63,13 +67,13 @@ var diagnoseCmd = &cobra.Command{ f, _ := os.Open(lockFilePath) defer f.Close() - packageManagerStrategy, ok := DoctorWithStrategy[o.packageManagerName] + doctor, ok := doctors[o.packageManagerName] if !ok { log.Fatal("unknown package manager") } - doctor := NewDoctor(packageManagerStrategy) - diagnoses := doctor.Diagnose(f, MAX_YEAR_TO_BE_BLANK) + department := NewDepartment(doctor) + diagnoses := department.Diagnose(f, MAX_YEAR_TO_BE_BLANK) err := Report(diagnoses) if err != nil { os.Exit(1) diff --git a/cmd/diagnose_test.go b/cmd/diagnose_test.go index 0e84b1e..7c72e65 100644 --- a/cmd/diagnose_test.go +++ b/cmd/diagnose_test.go @@ -59,7 +59,7 @@ func TestDiagnose(t *testing.T) { require.NoError(t, err) defer f.Close() - doctor := NewDoctor(NewBundlerStrategy()) + doctor := NewDepartment(NewBundlerDoctor()) diagnoses := doctor.Diagnose(f, 2) assert.Equal(t, expect, diagnoses) }) diff --git a/cmd/ruby/bundler/bundler.go b/cmd/ruby/bundler/bundler.go deleted file mode 100644 index 1712959..0000000 --- a/cmd/ruby/bundler/bundler.go +++ /dev/null @@ -1,94 +0,0 @@ -package cmd - -import ( - "fmt" - - "github.com/aquasecurity/go-dep-parser/pkg/io" - "github.com/aquasecurity/go-dep-parser/pkg/ruby/bundler" - "github.com/kyoshidajp/dep-doctor/cmd" - "github.com/kyoshidajp/dep-doctor/cmd/github" -) - -const GITHUB_SEARCH_REPO_COUNT_PER_ONCE = 20 - -func getNameWithOwners(r io.ReadSeekerAt) []github.NameWithOwner { - var nameWithOwners []github.NameWithOwner - p := &bundler.Parser{} - libs, _, _ := p.Parse(r) - - for _, lib := range libs { - fmt.Printf("%s\n", lib.Name) - - githubUrl, err := cmd.FetchFromRubyGems(lib.Name) - if err != nil { - continue - } - - repo, err := github.ParseGitHubUrl(githubUrl) - - if err != nil { - nameWithOwners = append(nameWithOwners, - github.NameWithOwner{ - PackageName: lib.Name, - CanSearch: false, - }, - ) - } else { - nameWithOwners = append(nameWithOwners, - github.NameWithOwner{ - Repo: repo.Repo, - Owner: repo.Owner, - PackageName: lib.Name, - CanSearch: true, - }, - ) - } - } - - return nameWithOwners -} - -type BundlerDoctor struct { -} - -func (d *BundlerDoctor) Diagnose(r io.ReadSeekerAt, year int) map[string]cmd.Diagnosis { - diagnoses := make(map[string]cmd.Diagnosis) - slicedNameWithOwners := [][]github.NameWithOwner{} - nameWithOwners := getNameWithOwners(r) - sliceSize := len(nameWithOwners) - - for i := 0; i < sliceSize; i += GITHUB_SEARCH_REPO_COUNT_PER_ONCE { - end := i + GITHUB_SEARCH_REPO_COUNT_PER_ONCE - if sliceSize < end { - end = sliceSize - } - slicedNameWithOwners = append(slicedNameWithOwners, nameWithOwners[i:end]) - } - - for _, nameWithOwners := range slicedNameWithOwners { - repos := github.FetchFromGitHub(nameWithOwners) - for _, r := range repos { - diagnosis := cmd.Diagnosis{ - Name: r.Name, - Url: r.Url, - Archived: r.Archived, - Diagnosed: true, - IsActive: r.IsActive(year), - } - diagnoses[r.Name] = diagnosis - } - } - - for _, nameWithOwner := range nameWithOwners { - if nameWithOwner.CanSearch { - continue - } - - diagnosis := cmd.Diagnosis{ - Name: nameWithOwner.PackageName, - Diagnosed: false, - } - diagnoses[nameWithOwner.PackageName] = diagnosis - } - return diagnoses -} diff --git a/cmd/yarn.go b/cmd/yarn.go index ea3bd67..f5da87d 100644 --- a/cmd/yarn.go +++ b/cmd/yarn.go @@ -19,7 +19,14 @@ type NodejsRegistryResponse struct { } } -func FetchFromNodejs(name string) string { +type YarnDoctor struct { +} + +func NewYarnDoctor() *YarnDoctor { + return &YarnDoctor{} +} + +func (d *YarnDoctor) fetchURLFromRepository(name string) (string, error) { url := fmt.Sprintf(NODEJS_REGISTRY_API, name) req, _ := http.NewRequest(http.MethodGet, url, nil) client := new(http.Client) @@ -29,23 +36,16 @@ func FetchFromNodejs(name string) string { var NodejsRegistryResponse NodejsRegistryResponse err := json.Unmarshal(body, &NodejsRegistryResponse) if err != nil { - return "" + return "", nil } - return NodejsRegistryResponse.Repository.Url + return NodejsRegistryResponse.Repository.Url, nil } -type YarnStrategy struct { -} - -func NewYarnStrategy() *YarnStrategy { - return &YarnStrategy{} -} - -func (s *YarnStrategy) Diagnose(r parser_io.ReadSeekerAt, year int) map[string]Diagnosis { +func (d *YarnDoctor) Diagnose(r parser_io.ReadSeekerAt, year int) map[string]Diagnosis { diagnoses := make(map[string]Diagnosis) slicedNameWithOwners := [][]github.NameWithOwner{} - nameWithOwners := s.getNameWithOwners(r) + nameWithOwners := d.NameWithOwners(r) sliceSize := len(nameWithOwners) for i := 0; i < sliceSize; i += GITHUB_SEARCH_REPO_COUNT_PER_ONCE { @@ -84,16 +84,14 @@ func (s *YarnStrategy) Diagnose(r parser_io.ReadSeekerAt, year int) map[string]D return diagnoses } -func (s *YarnStrategy) getNameWithOwners(r parser_io.ReadSeekerAt) []github.NameWithOwner { +func (d *YarnDoctor) NameWithOwners(r parser_io.ReadSeekerAt) []github.NameWithOwner { var nameWithOwners []github.NameWithOwner libs, _, _ := yarn.NewParser().Parse(r) for _, lib := range libs { fmt.Printf("%s\n", lib.Name) - githubUrl := FetchFromNodejs(lib.Name) - repo, err := github.ParseGitHubUrl(githubUrl) - + githubUrl, err := d.fetchURLFromRepository(lib.Name) if err != nil { nameWithOwners = append(nameWithOwners, github.NameWithOwner{ @@ -101,16 +99,28 @@ func (s *YarnStrategy) getNameWithOwners(r parser_io.ReadSeekerAt) []github.Name CanSearch: false, }, ) - } else { + continue + } + + repo, err := github.ParseGitHubUrl(githubUrl) + if err != nil { nameWithOwners = append(nameWithOwners, github.NameWithOwner{ - Repo: repo.Repo, - Owner: repo.Owner, PackageName: lib.Name, - CanSearch: true, + CanSearch: false, }, ) + continue } + + nameWithOwners = append(nameWithOwners, + github.NameWithOwner{ + Repo: repo.Repo, + Owner: repo.Owner, + PackageName: lib.Name, + CanSearch: true, + }, + ) } return nameWithOwners diff --git a/cmd/yarn_test.go b/cmd/yarn_test.go index 4bf377d..bbdacbb 100644 --- a/cmd/yarn_test.go +++ b/cmd/yarn_test.go @@ -7,18 +7,14 @@ import ( "github.com/stretchr/testify/assert" ) -func TestFetchFromRubyGems(t *testing.T) { +func TestYarnDoctor_fetchURLFromRepository(t *testing.T) { tests := []struct { name string gem_name string }{ { name: "source_code_uri exists", - gem_name: "rails", - }, - { - name: "no source_code_uri, but homepage_uri exists", - gem_name: "minitest", + gem_name: "react", }, } expects := []struct { @@ -27,17 +23,14 @@ func TestFetchFromRubyGems(t *testing.T) { }{ { name: "source_code_uri exists", - url: "https://github.com/rails/rails", - }, - { - name: "no source_code_uri, but homepage_uri exists", - url: "https://github.com/minitest/minitest", + url: "git+https://github.com/facebook/react", }, } for i, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r, _ := FetchFromRubyGems(tt.gem_name) + s := YarnDoctor{} + r, _ := s.fetchURLFromRepository(tt.gem_name) expect := expects[i] assert.Equal(t, true, strings.HasPrefix(r, expect.url)) })