fix proposal: ByteStream consumer can write to interface{}

ByteStreamConsumer may write into an interface which underlying type is []byte or string. * fixes go-openapi#167 This PR affects only the ByteStreamConsumer. Obviously, there is a lot of commonality with the TextConsumer, which could be augmented the same way in a follow-up. Also the newly proposed type switch could benefit the Producer. Signed-off-by: Frederic BIDON <[email protected]>
fredbi · Dec 8, 2023 · e1af0d1 · e1af0d1
1 parent 1af6e90
commit e1af0d1
Show file tree

Hide file tree

Showing 2 changed files with 130 additions and 55 deletions.
diff --git a/bytestream.go b/bytestream.go
@@ -39,7 +39,7 @@ type byteStreamOpts struct {
 }
 
 // ByteStreamConsumer creates a consumer for byte streams,
-// takes a Writer/BinaryUnmarshaler interface or binary slice by reference,
+// takes a Writer/BinaryUnmarshaler interface, string or binary slice by reference,
 // and reads from the provided reader
 func ByteStreamConsumer(opts ...byteStreamOpt) Consumer {
 	var vals byteStreamOpts
@@ -51,15 +51,19 @@ func ByteStreamConsumer(opts ...byteStreamOpt) Consumer {
 		if reader == nil {
 			return errors.New("ByteStreamConsumer requires a reader") // early exit
 		}
+		if data == nil {
+			return errors.New("nil destination for ByteStreamConsumer")
+		}
 
-		close := defaultCloser //nolint:revive,predeclared
+		closer := defaultCloser
 		if vals.Close {
 			if cl, ok := reader.(io.Closer); ok {
-				close = cl.Close //nolint:revive
+				closer = cl.Close
 			}
 		}
-		//nolint:errcheck // closing a reader wouldn't fail.
-		defer close()
+		defer func() {
+			_ = closer()
+		}()
 
 		if wrtr, ok := data.(io.Writer); ok {
 			_, err := io.Copy(wrtr, reader)
@@ -73,22 +77,46 @@ func ByteStreamConsumer(opts ...byteStreamOpt) Consumer {
 		}
 		b := buf.Bytes()
 
-		if bu, ok := data.(encoding.BinaryUnmarshaler); ok {
-			return bu.UnmarshalBinary(b)
-		}
+		switch dest := data.(type) {
+		case encoding.BinaryUnmarshaler:
+			return dest.UnmarshalBinary(b)
+		case *string:
+			*dest = string(b)
+
+			return nil
+		case *[]byte:
+			*dest = b
+
+			return nil
+		case *any:
+			switch (*dest).(type) {
+			case string:
+				*dest = string(b)
+
+				return nil
+
+			case []byte:
+				*dest = b
 
-		if data != nil {
-			if str, ok := data.(*string); ok {
-				*str = string(b)
 				return nil
 			}
-		}
+		default:
+			// check for the underlying type to be []byte or string
+			ptr := reflect.TypeOf(data)
+			if ptr.Kind() != reflect.Ptr {
+				break
+			}
 
-		if t := reflect.TypeOf(data); data != nil && t.Kind() == reflect.Ptr {
 			v := reflect.Indirect(reflect.ValueOf(data))
-			if t = v.Type(); t.Kind() == reflect.Slice && t.Elem().Kind() == reflect.Uint8 {
+			t := v.Type()
+
+			switch {
+			case t.Kind() == reflect.Slice && t.Elem().Kind() == reflect.Uint8:
 				v.SetBytes(b)
 				return nil
+			case t.Kind() == reflect.String:
+				v.SetString(string(b))
+				return nil
 			}
 		}
 

diff --git a/bytestream_test.go b/bytestream_test.go
@@ -12,48 +12,95 @@ import (
 )
 
 func TestByteStreamConsumer(t *testing.T) {
-	cons := ByteStreamConsumer()
-
 	const expected = "the data for the stream to be sent over the wire"
-
-	// can consume as a Writer
-	var b bytes.Buffer
-	require.NoError(t, cons.Consume(bytes.NewBufferString(expected), &b))
-	assert.Equal(t, expected, b.String())
-
-	// can consume as a string
-	var s string
-	require.NoError(t, cons.Consume(bytes.NewBufferString(expected), &s))
-	assert.Equal(t, expected, s)
-
-	// can consume as an UnmarshalBinary
-	var bu binaryUnmarshalDummy
-	require.NoError(t, cons.Consume(bytes.NewBufferString(expected), &bu))
-	assert.Equal(t, expected, bu.str)
-
-	// can consume as a binary slice
-	var bs []byte
-	require.NoError(t, cons.Consume(bytes.NewBufferString(expected), &bs))
-	assert.Equal(t, expected, string(bs))
-
-	type binarySlice []byte
-	var bs2 binarySlice
-	require.NoError(t, cons.Consume(bytes.NewBufferString(expected), &bs2))
-	assert.Equal(t, expected, string(bs2))
-
-	// passing in a nilslice wil result in an error
-	var ns *[]byte
-	require.Error(t, cons.Consume(bytes.NewBufferString(expected), &ns))
-
-	// passing in nil wil result in an error as well
-	require.Error(t, cons.Consume(bytes.NewBufferString(expected), nil))
-
-	// a reader who results in an error, will make it fail
-	require.Error(t, cons.Consume(new(nopReader), &bu))
-	require.Error(t, cons.Consume(new(nopReader), &bs))
-
-	// the readers can also not be nil
-	require.Error(t, cons.Consume(nil, &bs))
+	consumer := ByteStreamConsumer()
+
+	t.Run("can consume as a Writer", func(t *testing.T) {
+		var dest bytes.Buffer
+		require.NoError(t, consumer.Consume(bytes.NewBufferString(expected), &dest))
+		assert.Equal(t, expected, dest.String())
+	})
+
+	t.Run("can consume as a string", func(t *testing.T) {
+		var dest string
+		require.NoError(t, consumer.Consume(bytes.NewBufferString(expected), &dest))
+		assert.Equal(t, expected, dest)
+	})
+
+	t.Run("can consume as an UnmarshalBinary", func(t *testing.T) {
+		var dest binaryUnmarshalDummy
+		require.NoError(t, consumer.Consume(bytes.NewBufferString(expected), &dest))
+		assert.Equal(t, expected, dest.str)
+	})
+
+	t.Run("can consume as a binary slice", func(t *testing.T) {
+		var dest []byte
+		require.NoError(t, consumer.Consume(bytes.NewBufferString(expected), &dest))
+		assert.Equal(t, expected, string(dest))
+	})
+
+	t.Run("can consume as a type, with underlying as a binary slice", func(t *testing.T) {
+		type binarySlice []byte
+		var dest binarySlice
+		require.NoError(t, consumer.Consume(bytes.NewBufferString(expected), &dest))
+		assert.Equal(t, expected, string(dest))
+	})
+
+	t.Run("can consume as a type, with underlying as a string", func(t *testing.T) {
+		type aliasedString string
+		var dest aliasedString
+		require.NoError(t, consumer.Consume(bytes.NewBufferString(expected), &dest))
+		assert.Equal(t, expected, string(dest))
+	})
+
+	t.Run("can consume as an interface with underlying type []byte", func(t *testing.T) {
+		var dest interface{} = []byte{}
+		require.NoError(t, consumer.Consume(bytes.NewBufferString(expected), &dest))
+		asBytes, ok := dest.([]byte)
+		require.True(t, ok)
+		assert.Equal(t, expected, string(asBytes))
+	})
+
+	t.Run("can consume as an interface with underlying type string", func(t *testing.T) {
+		var dest interface{} = "x"
+		require.NoError(t, consumer.Consume(bytes.NewBufferString(expected), &dest))
+		asString, ok := dest.(string)
+		require.True(t, ok)
+		assert.Equal(t, expected, asString)
+	})
+
+	t.Run("error cases", func(t *testing.T) {
+		t.Run("passing in a nil slice will result in an error", func(t *testing.T) {
+			var dest *[]byte
+			require.Error(t, consumer.Consume(bytes.NewBufferString(expected), &dest))
+		})
+
+		t.Run("passing a non-pointer will result in an error", func(t *testing.T) {
+			var dest []byte
+			require.Error(t, consumer.Consume(bytes.NewBufferString(expected), dest))
+		})
+
+		t.Run("passing in nil destination result in an error", func(t *testing.T) {
+			require.Error(t, consumer.Consume(bytes.NewBufferString(expected), nil))
+		})
+
+		t.Run("a reader who results in an error, will make it fail", func(t *testing.T) {
+			t.Run("binaryUnmarshal case", func(t *testing.T) {
+				var dest binaryUnmarshalDummy
+				require.Error(t, consumer.Consume(new(nopReader), &dest))
+			})
+
+			t.Run("[]byte case", func(t *testing.T) {
+				var dest []byte
+				require.Error(t, consumer.Consume(new(nopReader), &dest))
+			})
+		})
+
+		t.Run("the reader cannot be nil", func(t *testing.T) {
+			var dest []byte
+			require.Error(t, consumer.Consume(nil, &dest))
+		})
+	})
 }
 
 type binaryUnmarshalDummy struct {