GODRIVER-1020 Allow empty JSON string to decode into ObjectID (#384)

Author: Divjot Arora

bson/primitive/objectid.go

@@ -93,11 +93,18 @@ func (id ObjectID) MarshalJSON() ([]byte, error) {
 	return json.Marshal(id.Hex())
 }
 
-// UnmarshalJSON populates the byte slice with the ObjectID. If the byte slice is 64 bytes long, it
+// UnmarshalJSON populates the byte slice with the ObjectID. If the byte slice is 24 bytes long, it
 // will be populated with the hex representation of the ObjectID. If the byte slice is twelve bytes
-// long, it will be populated with the BSON representation of the ObjectID. Otherwise, it will
-// return an error.
+// long, it will be populated with the BSON representation of the ObjectID. This method also accepts empty strings and
+// decodes them as NilObjectID. For any other inputs, an error will be returned.
 func (id *ObjectID) UnmarshalJSON(b []byte) error {
+	// Ignore "null" to keep parity with the standard library. Decoding a JSON null into a non-pointer ObjectID field
+	// will leave the field unchanged. For pointer values, encoding/json will set the pointer to nil and will not
+	// enter the UnmarshalJSON hook.
+	if string(b) == "null" {
+		return nil
+	}
+
 	var err error
 	switch len(b) {
 	case 12:
@@ -125,6 +132,12 @@ func (id *ObjectID) UnmarshalJSON(b []byte) error {
 			}
 		}
 
+		// An empty string is not a valid ObjectID, but we treat it as a special value that decodes as NilObjectID.
+		if len(str) == 0 {
+			copy(id[:], NilObjectID[:])
+			return nil
+		}
+
 		if len(str) != 24 {
 			return fmt.Errorf("cannot unmarshal into an ObjectID, the length must be 24 but it is %d", len(str))
 		}

bson/primitive/objectid_test.go

@@ -7,13 +7,15 @@
 package primitive
 
 import (
-	"testing"
-
 	"encoding/binary"
 	"encoding/hex"
+	"encoding/json"
+	"fmt"
+	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
+	"go.mongodb.org/mongo-driver/internal/testutil/assert"
 )
 
 func TestNew(t *testing.T) {
@@ -148,3 +150,33 @@ func TestCounterOverflow(t *testing.T) {
 	NewObjectID()
 	require.Equal(t, uint32(0), objectIDCounter)
 }
+
+func TestObjectID_UnmarshalJSON(t *testing.T) {
+	oid := NewObjectID()
+
+	hexJSON := fmt.Sprintf(`{"foo": %q}`, oid.Hex())
+	extJSON := fmt.Sprintf(`{"foo": {"$oid": %q}}`, oid.Hex())
+	emptyStringJSON := `{"foo": ""}`
+	nullJSON := `{"foo": null}`
+
+	testCases := []struct {
+		name       string
+		jsonString string
+		expected   ObjectID
+	}{
+		{"hex bytes", hexJSON, oid},
+		{"extended JSON", extJSON, oid},
+		{"empty string", emptyStringJSON, NilObjectID},
+		{"null", nullJSON, NilObjectID},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var got map[string]ObjectID
+			err := json.Unmarshal([]byte(tc.jsonString), &got)
+			assert.Nil(t, err, "Unmarshal error: %v", err)
+
+			gotOid := got["foo"]
+			assert.Equal(t, tc.expected, gotOid, "expected ObjectID %s, got %s", tc.expected, gotOid)
+		})
+	}
+}