fix: reinstate default bundle create behaviour (#116)

Signed-off-by: Andrew Robinson Hodges <[email protected]>

Author: andrewrobinsonhodges-snyk

README.md

@@ -84,7 +84,7 @@ Implement the `config.Config` interface to configure the Snyk Code API client fr
 
 ### Code Scanner
 
-Use the Code Scanner to trigger a scan for a Snyk Code workspace using the Bundle Manager created above.
+Use the Code Scanner to trigger a scan for a Snyk Code workspace using the Bundle Manager.
 
 The Code Scanner exposes a `UploadAndAnalyze` function, which can be used like this:
 

bundle/bundle.go

@@ -145,7 +145,7 @@ func NewBatchFromRawContent(documents map[string][]byte) (*Batch, error) {
 	bundleFiles := make(map[string]deepcode.BundleFile)
 
 	for key, rawData := range documents {
-		bundleFile, err := deepcode.BundleFileFrom(rawData)
+		bundleFile, err := deepcode.BundleFileFrom(rawData, true)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create file from raw data: %v", err)
 		}

bundle/bundle_manager.go

@@ -50,6 +50,13 @@ type BundleManager interface {
 		changedFiles map[string]bool,
 	) (bundle Bundle, err error)
 
+	// CreateEmpty does not include the file contents in the bundle.
+	CreateEmpty(ctx context.Context,
+		rootPath string,
+		filePaths <-chan string,
+		changedFiles map[string]bool,
+	) (bundle Bundle, err error)
+
 	Upload(
 		ctx context.Context,
 		requestId string,
@@ -76,11 +83,31 @@ func NewBundleManager(
 	}
 }
 
-func (b *bundleManager) Create(ctx context.Context,
+func (b *bundleManager) Create(
+	ctx context.Context,
 	requestId string,
 	rootPath string,
 	filePaths <-chan string,
 	changedFiles map[string]bool,
+) (bundle Bundle, err error) {
+	return b.create(ctx, rootPath, filePaths, changedFiles, true)
+}
+
+func (b *bundleManager) CreateEmpty(
+	ctx context.Context,
+	rootPath string,
+	filePaths <-chan string,
+	changedFiles map[string]bool,
+) (bundle Bundle, err error) {
+	return b.create(ctx, rootPath, filePaths, changedFiles, false)
+}
+
+func (b *bundleManager) create(
+	ctx context.Context,
+	rootPath string,
+	filePaths <-chan string,
+	changedFiles map[string]bool,
+	includeFileContents bool,
 ) (bundle Bundle, err error) {
 	span := b.instrumentor.StartSpan(ctx, "code.createBundle")
 	defer b.instrumentor.Finish(span)
@@ -129,7 +156,7 @@ func (b *bundleManager) Create(ctx context.Context,
 		}
 		relativePath = util.EncodePath(relativePath)
 
-		bundleFile, fileErr := deepcode.BundleFileFrom(fileContent)
+		bundleFile, fileErr := deepcode.BundleFileFrom(fileContent, includeFileContents)
 		if fileErr != nil {
 			b.logger.Error().Err(err).Str("filePath", absoluteFilePath).Msg("Error creating bundle file")
 		}

bundle/bundle_test.go

@@ -25,16 +25,18 @@ import (
 	"os"
 	"testing"
 
+	"golang.org/x/net/html/charset"
+
 	"github.com/golang/mock/gomock"
 	"github.com/rs/zerolog"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"golang.org/x/net/html/charset"
 
 	"github.com/snyk/code-client-go/bundle"
 	"github.com/snyk/code-client-go/internal/deepcode"
 	deepcodeMocks "github.com/snyk/code-client-go/internal/deepcode/mocks"
+	"github.com/snyk/code-client-go/internal/util"
 	"github.com/snyk/code-client-go/observability/mocks"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var bundleWithFiles = bundle.NewBatch(map[string]deepcode.BundleFile{"file": {}})
@@ -140,7 +142,7 @@ func Test_RawContentBatch(t *testing.T) {
 	t.Run("create a batch from raw content and upload the bundle", func(t *testing.T) {
 		ctrl := gomock.NewController(t)
 		mockSnykCodeClient := deepcodeMocks.NewMockDeepcodeClient(ctrl)
-		mockSnykCodeClient.EXPECT().ExtendBundle(gomock.Any(), "testBundleHash", bundleFilePartialMatcher{expectedKey: "hello", expectedContent: ""}, []string{}).Return("newBundleHash", []string{}, nil).Times(1)
+		mockSnykCodeClient.EXPECT().ExtendBundle(gomock.Any(), "testBundleHash", bundleFilePartialMatcher{expectedKey: "hello", expectedContent: "world"}, []string{}).Return("newBundleHash", []string{}, nil).Times(1)
 
 		mockSpan := mocks.NewMockSpan(ctrl)
 		mockSpan.EXPECT().Context().AnyTimes()
@@ -162,7 +164,7 @@ func Test_RawContentBatch(t *testing.T) {
 func Test_BundleEncoding(t *testing.T) {
 	t.Run("utf-8 encoded content", func(t *testing.T) {
 		content := []byte("hello")
-		bundleFile, err := deepcode.BundleFileFrom(content)
+		bundleFile, err := deepcode.BundleFileFrom(content, false)
 		assert.NoError(t, err)
 
 		ExpectedShaSum := sha256.Sum256(content)
@@ -173,7 +175,7 @@ func Test_BundleEncoding(t *testing.T) {
 		content, err := os.ReadFile("testdata/rshell_font.php")
 		assert.NoError(t, err)
 
-		bundleFile, err := deepcode.BundleFileFrom(content)
+		bundleFile, err := deepcode.BundleFileFrom(content, false)
 		assert.NoError(t, err)
 
 		byteReader := bytes.NewReader(content)
@@ -183,3 +185,27 @@ func Test_BundleEncoding(t *testing.T) {
 		assert.Equal(t, hex.EncodeToString(ExpectedShaSum[:]), bundleFile.Hash)
 	})
 }
+
+func Test_BundleFileContent(t *testing.T) {
+	t.Run("include file contents", func(t *testing.T) {
+		content := []byte("hello")
+		bundleFile, err := deepcode.BundleFileFrom(content, true)
+		assert.NoError(t, err)
+
+		utf8Content, err := util.ConvertToUTF8(content)
+		assert.NoError(t, err)
+
+		assert.Equal(t, string(utf8Content), bundleFile.Content)
+		assert.Equal(t, len(content), bundleFile.ContentSize)
+	})
+
+	t.Run("exclude file contents", func(t *testing.T) {
+		content := []byte("hello")
+		bundleFile, err := deepcode.BundleFileFrom(content, false)
+		assert.NoError(t, err)
+
+		assert.Equal(t, "", bundleFile.Content)
+		// Note that we still expect the bundle to indicate the expected final size.
+		assert.Equal(t, len(content), bundleFile.ContentSize)
+	})
+}

bundle/mocks/bundle_manager.go

@@ -25,11 +25,26 @@ type BundleFile struct {
 	ContentSize int    `json:"-"`
 }
 
-func BundleFileFrom(content []byte) (BundleFile, error) {
+func BundleFileFrom(content []byte, includeContent bool) (BundleFile, error) {
 	hash, err := util.Hash(content)
+
+	// We can either create the bundleFile empty and enrich it with content later, or include the content now.
+	// Creating empty avoids keeping the file contents in memory, so improves performance if we don't need access to the
+	// contents right away.
+	bundleFileContent := ""
+	if includeContent {
+		utf8Content, convertErr := util.ConvertToUTF8(content)
+		if convertErr == nil {
+			bundleFileContent = string(utf8Content)
+		} else {
+			bundleFileContent = string(content)
+			err = convertErr
+		}
+	}
+
 	file := BundleFile{
 		Hash:        hash,
-		Content:     "", // We create the bundleFile empty, and enrich  with content later.
+		Content:     bundleFileContent,
 		ContentSize: len(content),
 	}
 	return file, err

internal/deepcode/helpers.go

@@ -215,7 +215,7 @@ func (c *codeScanner) Upload(
 		return nil, err
 	}
 
-	originalBundle, err := c.bundleManager.Create(ctx, requestId, target.GetPath(), files, changedFiles)
+	originalBundle, err := c.bundleManager.CreateEmpty(ctx, target.GetPath(), files, changedFiles)
 	err = c.checkCancellationOrLogError(ctx, target.GetPath(), err, "error creating bundle...")
 	if err != nil {
 		return nil, err

scan.go

@@ -44,9 +44,9 @@ import (
 func Test_UploadAndAnalyze(t *testing.T) {
 	baseDir, firstDocPath, secondDocPath, firstDocContent, secondDocContent := setupDocs(t)
 	docs := sliceToChannel([]string{firstDocPath, secondDocPath})
-	firstBundle, err := deepcode.BundleFileFrom(firstDocContent)
+	firstBundle, err := deepcode.BundleFileFrom(firstDocContent, false)
 	assert.NoError(t, err)
-	secondBundle, err := deepcode.BundleFileFrom(secondDocContent)
+	secondBundle, err := deepcode.BundleFileFrom(secondDocContent, false)
 	assert.NoError(t, err)
 
 	files := map[string]deepcode.BundleFile{
@@ -80,7 +80,7 @@ func Test_UploadAndAnalyze(t *testing.T) {
 			requestId := uuid.NewString()
 			mockBundle := bundle.NewBundle(deepcodeMocks.NewMockDeepcodeClient(ctrl), mockInstrumentor, mockErrorReporter, &logger, "testRootPath", "", files, []string{}, []string{})
 			mockBundleManager := bundleMocks.NewMockBundleManager(ctrl)
-			mockBundleManager.EXPECT().Create(gomock.Any(), requestId, baseDir, gomock.Any(), map[string]bool{}).Return(mockBundle, nil)
+			mockBundleManager.EXPECT().CreateEmpty(gomock.Any(), baseDir, gomock.Any(), map[string]bool{}).Return(mockBundle, nil)
 			mockBundleManager.EXPECT().Upload(gomock.Any(), requestId, mockBundle, files).Return(mockBundle, nil)
 
 			codeScanner := codeclient.NewCodeScanner(
@@ -104,7 +104,7 @@ func Test_UploadAndAnalyze(t *testing.T) {
 			requestId := uuid.NewString()
 			mockBundle := bundle.NewBundle(deepcodeMocks.NewMockDeepcodeClient(ctrl), mockInstrumentor, mockErrorReporter, &logger, "testRootPath", uuid.NewString(), files, []string{}, []string{})
 			mockBundleManager := bundleMocks.NewMockBundleManager(ctrl)
-			mockBundleManager.EXPECT().Create(gomock.Any(), requestId, baseDir, gomock.Any(), map[string]bool{}).Return(mockBundle, nil)
+			mockBundleManager.EXPECT().CreateEmpty(gomock.Any(), baseDir, gomock.Any(), map[string]bool{}).Return(mockBundle, nil)
 			mockBundleManager.EXPECT().Upload(gomock.Any(), requestId, mockBundle, files).Return(mockBundle, nil)
 
 			codeScanner := codeclient.NewCodeScanner(
@@ -129,7 +129,7 @@ func Test_UploadAndAnalyze(t *testing.T) {
 			requestId := uuid.NewString()
 			mockBundle := bundle.NewBundle(deepcodeMocks.NewMockDeepcodeClient(ctrl), mockInstrumentor, mockErrorReporter, &logger, "testRootPath", uuid.NewString(), files, []string{}, []string{})
 			mockBundleManager := bundleMocks.NewMockBundleManager(ctrl)
-			mockBundleManager.EXPECT().Create(gomock.Any(), requestId, baseDir, gomock.Any(), map[string]bool{}).Return(mockBundle, nil)
+			mockBundleManager.EXPECT().CreateEmpty(gomock.Any(), baseDir, gomock.Any(), map[string]bool{}).Return(mockBundle, nil)
 			mockBundleManager.EXPECT().Upload(gomock.Any(), requestId, mockBundle, files).Return(mockBundle, nil)
 
 			mockAnalysisOrchestrator := mockAnalysis.NewMockAnalysisOrchestrator(ctrl)
@@ -166,7 +166,7 @@ func Test_UploadAndAnalyze(t *testing.T) {
 			requestId := uuid.NewString()
 			mockBundle := bundle.NewBundle(deepcodeMocks.NewMockDeepcodeClient(ctrl), mockInstrumentor, mockErrorReporter, &logger, "testRootPath", uuid.NewString(), files, []string{relativeChangedFile}, []string{})
 			mockBundleManager := bundleMocks.NewMockBundleManager(ctrl)
-			mockBundleManager.EXPECT().Create(gomock.Any(), requestId, baseDir, gomock.Any(), map[string]bool{}).Return(mockBundle, nil)
+			mockBundleManager.EXPECT().CreateEmpty(gomock.Any(), baseDir, gomock.Any(), map[string]bool{}).Return(mockBundle, nil)
 			mockBundleManager.EXPECT().Upload(gomock.Any(), requestId, mockBundle, files).Return(mockBundle, nil)
 
 			mockAnalysisOrchestrator := mockAnalysis.NewMockAnalysisOrchestrator(ctrl)