Remove recursive implementation from github file client get and its i…

…ntegration tests

github/client_repository_file.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 	"io/ioutil"
-	"strings"
 
 	"github.com/fluxcd/go-git-providers/gitprovider"
 	"github.com/google/go-github/v42/github"
@@ -35,36 +34,6 @@ type FileClient struct {
 	ref gitprovider.RepositoryRef
 }
 
-// if number of files retrieved exceed the max number of files, return slice only until the number of maxFiles with bool true for limiting succeeded
-func limitSliceSize(s []*gitprovider.CommitFile, size int) ([]*gitprovider.CommitFile, bool) {
-	if size != 0 && len(s) >= size {
-		return s[:size], true
-	}
-	return s, false
-}
-
-// getDirectoryFiles will make the recursive call to get files in sub directory of a dir type
-func (c *FileClient) getDirectoryFiles(ctx context.Context, path string, branch string, dir *github.RepositoryContent, files []*gitprovider.CommitFile, fileOpts gitprovider.FilesGetOptions) ([]*gitprovider.CommitFile, error) {
-
-	// stop recursive calls when level is the max level reached
-	if fileOpts.MaxDepth <= 1 {
-		return nil, nil
-	}
-
-	if !strings.HasSuffix(path, "/") {
-		path = path + "/"
-	}
-	subdirectoryPath := fmt.Sprintf("%s%s/", path, *dir.Name)
-
-	// recursive call for child directories to get their content
-	childMaxFiles := fileOpts.MaxFiles - len(files)
-	childOptFns := gitprovider.FilesGetOptions{Recursive: fileOpts.Recursive, MaxFiles: childMaxFiles, MaxDepth: fileOpts.MaxDepth - 1}
-	childFiles, err := c.Get(ctx, subdirectoryPath, branch, &childOptFns)
-
-	return childFiles, err
-
-}
-
 // Get fetches and returns the contents of a file or directory from a given branch and path with possible options of FilesGetOption
 // If recursive option is provided, the files are retrieved recursively from subdirectories of the base path.
 // If recursive and MaxDepth options are provided, the files are retrieved recursively from subdirectories until reaching the max depth of levels
@@ -96,26 +65,6 @@ func (c *FileClient) Get(ctx context.Context, path, branch string, optFns ...git
 
 	for _, file := range directoryContent {
 		filePath := file.Path
-		if *file.Type == "dir" {
-			if fileOpts.Recursive {
-				childFiles, err := c.getDirectoryFiles(ctx, path, branch, file, files, fileOpts)
-
-				if err != nil {
-					return nil, err
-				}
-				if childFiles == nil {
-					continue
-				}
-
-				files = append(files, childFiles...)
-				files, maxFilesReached := limitSliceSize(files, fileOpts.MaxFiles)
-				if maxFilesReached {
-					return files, nil
-				}
-			}
-			continue
-
-		}
 		output, _, err := c.c.Client().Repositories.DownloadContents(ctx, c.ref.GetIdentity(), c.ref.GetRepository(), *filePath, opts)
 		if err != nil {
 			return nil, err
@@ -133,10 +82,6 @@ func (c *FileClient) Get(ctx context.Context, path, branch string, optFns ...git
 			Path:    filePath,
 			Content: &contentStr,
 		})
-		files, limitSuccess := limitSliceSize(files, fileOpts.MaxFiles)
-		if limitSuccess {
-			return files, nil
-		}
 
 	}
 

github/integration_test.go

@@ -562,102 +562,6 @@ var _ = Describe("GitHub Provider", func() {
 
 	})
 
-	It("should be possible to download files from path and branch specified with nested directory and max options", func() {
-
-		userRepoRef := newUserRepoRef(testUser, testUserRepoName)
-
-		userRepo, err := c.UserRepositories().Get(ctx, userRepoRef)
-		Expect(err).ToNot(HaveOccurred())
-
-		defaultBranch := userRepo.Get().DefaultBranch
-
-		path0 := "clustersDir/cluster/machine.yaml"
-		content0 := "machine yaml content"
-		path1 := "clustersDir/cluster/machine1.yaml"
-		content1 := "machine1 yaml content"
-		path2 := "clustersDir/cluster2/clusterSubDir/machine2.yaml"
-		content2 := "machine2 yaml content"
-
-		files := []gitprovider.CommitFile{
-			{
-				Path:    &path0,
-				Content: &content0,
-			},
-			{
-				Path:    &path1,
-				Content: &content1,
-			},
-			{
-				Path:    &path2,
-				Content: &content2,
-			},
-		}
-
-		commitFiles := make([]gitprovider.CommitFile, 0)
-		for _, file := range files {
-			path := file.Path
-			content := file.Content
-			commitFiles = append(commitFiles, gitprovider.CommitFile{
-				Path:    path,
-				Content: content,
-			})
-		}
-
-		_, err = userRepo.Commits().Create(ctx, *defaultBranch, "added config files", commitFiles)
-		Expect(err).ToNot(HaveOccurred())
-
-		//Recursive option check
-		options := gitprovider.FilesGetOptions{Recursive: *gitprovider.BoolVar(true)}
-		downloadedFiles, err := userRepo.Files().Get(ctx, "clustersDir", *defaultBranch, &options)
-		Expect(err).ToNot(HaveOccurred())
-		for ind, downloadedFile := range downloadedFiles {
-			Expect(*downloadedFile).To(Equal(files[ind]))
-		}
-
-		//Max depth option check
-		//should be files in 2 levels: machine.yaml and machine1.yaml in level 2 (level 1 has cluster and cluster2 dirs)
-		options = gitprovider.FilesGetOptions{Recursive: *gitprovider.BoolVar(true), MaxDepth: 2}
-		downloadedFiles, err = userRepo.Files().Get(ctx, "clustersDir", *defaultBranch, &options)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(downloadedFiles).To(HaveLen(2))
-
-		// Max files option check with recursion
-		// should be 1 file :machine.yaml
-		options = gitprovider.FilesGetOptions{Recursive: *gitprovider.BoolVar(true), MaxDepth: 2, MaxFiles: 1}
-		downloadedFiles, err = userRepo.Files().Get(ctx, "clustersDir", *defaultBranch, &options)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(downloadedFiles).To(HaveLen(1))
-
-		// Max files option check with no recursion (base level)
-		// should be 0
-		options = gitprovider.FilesGetOptions{MaxFiles: 2}
-		downloadedFiles, err = userRepo.Files().Get(ctx, "clustersDir", *defaultBranch, &options)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(downloadedFiles).To(HaveLen(0))
-
-		// Recursive option ,MaxDepth 3, all files check
-		// should be 3
-		options = gitprovider.FilesGetOptions{Recursive: *gitprovider.BoolVar(true), MaxDepth: 3}
-		downloadedFiles, err = userRepo.Files().Get(ctx, "clustersDir", *defaultBranch, &options)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(downloadedFiles).To(HaveLen(3))
-
-		// Recursive option,MaxDepth 2,and MaxFiles 2
-		// should be 2
-		options = gitprovider.FilesGetOptions{Recursive: *gitprovider.BoolVar(true), MaxDepth: 2, MaxFiles: 2}
-		downloadedFiles, err = userRepo.Files().Get(ctx, "clustersDir", *defaultBranch, &options)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(downloadedFiles).To(HaveLen(2))
-
-		// Recursive option,MaxDepth 3,and MaxFiles 2
-		// should be 2
-		options = gitprovider.FilesGetOptions{Recursive: *gitprovider.BoolVar(true), MaxDepth: 3, MaxFiles: 2}
-		downloadedFiles, err = userRepo.Files().Get(ctx, "clustersDir", *defaultBranch, &options)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(downloadedFiles).To(HaveLen(2))
-
-	})
-
 	AfterSuite(func() {
 		if os.Getenv("SKIP_CLEANUP") == "1" {
 			return

gitprovider/options.go

@@ -81,8 +81,6 @@ func (opts *RepositoryCreateOptions) ValidateOptions() error {
 // FilesGetOptions specifies optional options when fetcing files.
 type FilesGetOptions struct {
 	Recursive bool
-	MaxFiles  int
-	MaxDepth  int
 }
 
 type FilesGetOption interface {
@@ -94,10 +92,4 @@ func (opts *FilesGetOptions) ApplyFilesGetOptions(target *FilesGetOptions) {
 	if opts.Recursive == true {
 		target.Recursive = opts.Recursive
 	}
-	if opts.MaxFiles > 0 {
-		target.MaxFiles = opts.MaxFiles
-	}
-	if opts.MaxDepth > 0 {
-		target.MaxDepth = opts.MaxDepth
-	}
 }