Revert "refactor(http): preserve response body without copying in multiple services" (33130841) · Commits · GitLab.org / API / client-go

dependency_list_export.go

+4 −3

Original line number	Diff line number	Diff line
		package gitlab

		import (
		"bytes"
		"fmt"
		"io"
		"net/http"
		@@ -107,11 +108,11 @@ func (s *DependencyListExportService) DownloadDependencyListExport(id int64, opt
		return nil, nil, err
		}

		preserver := &bodyPreserver{}
		resp, err := s.client.Do(req, preserver)
		var sbomBuffer bytes.Buffer
		resp, err := s.client.Do(req, &sbomBuffer)
		if err != nil {
		return nil, resp, err
		}

		return preserver.body, resp, nil
		return &sbomBuffer, resp, nil
		}

dependency_list_export_test.go

+4 −12

Original line number	Diff line number	Diff line
		package gitlab

		import (
		"bytes"
		"encoding/json"
		"io"
		"net/http"
		@@ -76,21 +77,12 @@ func TestDownloadDependencyListExport(t *testing.T) {

		sbomReader, _, err := client.DependencyListExport.DownloadDependencyListExport(5678)
		require.NoError(t, err)
		require.NotNil(t, sbomReader)

		expectedSbom, err := os.ReadFile("testdata/download_dependency_list_export.json")
		require.NoError(t, err)

		// GIVEN: A reader is returned
		// WHEN: We read the content
		gotContent, err := io.ReadAll(sbomReader)
		require.NoError(t, err)

		// THEN: The content should match the expected SBOM
		require.Equal(t, expectedSbom, gotContent)
		var want bytes.Buffer
		want.Write(expectedSbom)

		// Clean up: Close the reader if it's a ReadCloser
		if rc, ok := sbomReader.(io.Closer); ok {
		rc.Close()
		}
		require.Equal(t, &want, sbomReader)
		}

gitlab.go

+6 −32

Original line number	Diff line number	Diff line
		@@ -1136,25 +1136,11 @@ func (r *Response) populateLinkValues() {
		}
		}

		// bodyPreserver is a special type that signals to Client.Do that we want to
		// preserve the response body without copying it. The body field will be set
		// to the actual response body, and the caller is responsible for closing it.
		type bodyPreserver struct {
		body io.ReadCloser
		}

		// Write implements io.Writer but does nothing. This prevents Client.Do from
		// copying the body content.
		func (bp *bodyPreserver) Write(_ []byte) (n int, err error) {
		return 0, nil
		}

		// Do sends an API request and returns the API response. The API response is
		// JSON decoded and stored in the value pointed to by v, or returned as an
		// error if an API error has occurred. If v implements the io.Writer
		// interface, the raw response body will be written to v, without attempting to
		// first decode it. If v is a *bodyPreserver, the response body is preserved
		// without copying and the caller is responsible for closing it.
		// first decode it.
		func (c Client) Do(req retryablehttp.Request, v any) (*Response, error) {
		// Wait will block until the limiter can obtain a new token.
		err := c.limiter.Wait(req.Context())
		@@ -1190,14 +1176,10 @@ func (c Client) Do(req retryablehttp.Request, v any) (*Response, error) {
		return nil, err
		}

		// Check if v is a bodyPreserver before setting up the defer
		preserver, isPreserver := v.(*bodyPreserver)
		if !isPreserver {
		defer func() {
		io.Copy(io.Discard, resp.Body)
		resp.Body.Close()
		}()
		}

		// If not yet configured, try to configure the rate limiter
		// using the response headers we just received. Fail silently
		@@ -1210,19 +1192,11 @@ func (c Client) Do(req retryablehttp.Request, v any) (*Response, error) {
		if err != nil {
		// Even though there was an error, we still return the response
		// in case the caller wants to inspect it further.
		// If using bodyPreserver, still need to close the body on error
		if isPreserver {
		io.Copy(io.Discard, resp.Body)
		resp.Body.Close()
		}
		return response, err
		}

		if v != nil {
		if isPreserver {
		// Preserve the body without copying
		preserver.body = resp.Body
		} else if w, ok := v.(io.Writer); ok {
		if w, ok := v.(io.Writer); ok {
		_, err = io.Copy(w, resp.Body)
		} else {
		err = json.NewDecoder(resp.Body).Decode(v)

secure_files.go

+4 −3

Original line number	Diff line number	Diff line
		@@ -14,6 +14,7 @@
		package gitlab

		import (
		"bytes"
		"fmt"
		"io"
		"net/http"
		@@ -179,13 +180,13 @@ func (s SecureFilesService) DownloadSecureFile(pid any, id int64, options ...Req
		return nil, nil, err
		}

		preserver := &bodyPreserver{}
		resp, err := s.client.Do(req, preserver)
		var file bytes.Buffer
		resp, err := s.client.Do(req, &file)
		if err != nil {
		return nil, resp, err
		}

		return preserver.body, resp, err
		return &file, resp, err
		}

		// RemoveSecureFile removes a project's secure file.

secure_files_test.go

+5 −18

Original line number	Diff line number	Diff line
		package gitlab

		import (
		"bytes"
		"fmt"
		"io"
		"net/http"
		"strings"
		"testing"
		"time"

		"github.com/stretchr/testify/assert"
		"github.com/stretchr/testify/require"
		)

		func TestSecureFiles_ListProjectSecureFiles(t *testing.T) {
		@@ -189,25 +188,13 @@ func TestSecureFiles_DownloadSecureFile(t *testing.T) {
		`))
		})

		wantContent := []byte("bar = baz")
		var want bytes.Buffer
		want.Write([]byte("bar = baz"))

		reader, resp, err := client.SecureFiles.DownloadSecureFile(1, 2)
		bytes, resp, err := client.SecureFiles.DownloadSecureFile(1, 2)
		assert.NoError(t, err)
		assert.NotNil(t, resp)
		require.NotNil(t, reader)

		// GIVEN: A reader is returned
		// WHEN: We read the content
		gotContent, err := io.ReadAll(reader)
		assert.NoError(t, err)

		// THEN: The content should match
		assert.Equal(t, wantContent, gotContent)

		// Clean up: Close the reader if it's a ReadCloser
		if rc, ok := reader.(io.Closer); ok {
		rc.Close()
		}
		assert.Equal(t, &want, bytes)
		}

		func TestSecureFiles_RemoveSecureFile(t *testing.T) {