Skip to content

Commit

Permalink
GODRIVER-2443 Make Distinct return a decodable struct (#1603)
Browse files Browse the repository at this point in the history
  • Loading branch information
prestonvasquez authored May 2, 2024
1 parent 3365ea1 commit c481fdf
Show file tree
Hide file tree
Showing 13 changed files with 211 additions and 127 deletions.
25 changes: 15 additions & 10 deletions internal/integration/collection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -874,24 +874,29 @@ func TestCollection(t *testing.T) {
}
})
mt.RunOpts("distinct", noClientOpts, func(mt *mtest.T) {
all := []interface{}{int32(1), int32(2), int32(3), int32(4), int32(5)}
last3 := []interface{}{int32(3), int32(4), int32(5)}
all := []int32{1, 2, 3, 4, 5}

testCases := []struct {
name string
filter bson.D
opts *options.DistinctOptions
expected []interface{}
name string
filter bson.D
opts *options.DistinctOptions
want []int32
}{
{"no options", bson.D{}, nil, all},
{"filter", bson.D{{"x", bson.D{{"$gt", 2}}}}, nil, last3},
{"filter", bson.D{{"x", bson.D{{"$gt", 2}}}}, nil, all[2:]},
{"options", bson.D{}, options.Distinct().SetMaxTime(5000000000), all},
}
for _, tc := range testCases {
mt.Run(tc.name, func(mt *mtest.T) {
initCollection(mt, mt.Coll)
res, err := mt.Coll.Distinct(context.Background(), "x", tc.filter, tc.opts)
assert.Nil(mt, err, "Distinct error: %v", err)
assert.Equal(mt, tc.expected, res, "expected result %v, got %v", tc.expected, res)
res := mt.Coll.Distinct(context.Background(), "x", tc.filter, tc.opts)
assert.Nil(mt, res.Err(), "Distinct error: %v", res.Err())

var got []int32
err := res.Decode(&got)
assert.NoError(t, err)

assert.EqualValues(mt, tc.want, got, "expected result %v, got %v", tc.want, got)
})
}
})
Expand Down
59 changes: 39 additions & 20 deletions internal/integration/crud_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ func executeListIndexes(mt *mtest.T, sess *mongo.Session, args bson.Raw) (*mongo
return mt.Coll.Indexes().List(context.Background())
}

func executeDistinct(mt *mtest.T, sess *mongo.Session, args bson.Raw) ([]interface{}, error) {
func executeDistinct(mt *mtest.T, sess *mongo.Session, args bson.Raw) (bson.RawArray, error) {
mt.Helper()

var fieldName string
Expand All @@ -627,16 +627,22 @@ func executeDistinct(mt *mtest.T, sess *mongo.Session, args bson.Raw) ([]interfa
}
}

var res *mongo.DistinctResult
if sess != nil {
var res []interface{}
err := mongo.WithSession(context.Background(), sess, func(sc context.Context) error {
var derr error
res, derr = mt.Coll.Distinct(sc, fieldName, filter, opts)
return derr
err := mongo.WithSession(context.Background(), sess, func(ctx context.Context) error {
res = mt.Coll.Distinct(ctx, fieldName, filter, opts)

return res.Err()
})
return res, err

if err != nil {
return nil, err
}
} else {
res = mt.Coll.Distinct(context.Background(), fieldName, filter, opts)
}
return mt.Coll.Distinct(context.Background(), fieldName, filter, opts)

return res.Raw()
}

func executeFindOneAndDelete(mt *mtest.T, sess *mongo.Session, args bson.Raw) *mongo.SingleResult {
Expand Down Expand Up @@ -1529,25 +1535,34 @@ func verifyDeleteResult(mt *mtest.T, res *mongo.DeleteResult, result interface{}
"deleted count mismatch; expected %v, got %v", expected.DeletedCount, res.DeletedCount)
}

func verifyDistinctResult(mt *mtest.T, actualResult []interface{}, expectedResult interface{}) {
func verifyDistinctResult(
mt *mtest.T,
got bson.RawArray,
want interface{},
) {
mt.Helper()

if expectedResult == nil {
if got == nil {
return
}

for i, expected := range expectedResult.(bson.A) {
actual := actualResult[i]
iExpected := getIntFromInterface(expected)
iActual := getIntFromInterface(actual)
assert.NotNil(mt, want, "expected want to be non-nil")

if iExpected != nil {
assert.NotNil(mt, iActual, "expected nil but got %v", iActual)
assert.Equal(mt, *iExpected, *iActual, "expected value %v but got %v", *iExpected, *iActual)
continue
arr, ok := want.(bson.A)
assert.True(mt, ok, "expected want to be a BSON array")

for i, iwant := range arr {
gotRawValue := got.Index(uint(i))

iwantType, iwantBytes, err := bson.MarshalValue(iwant)
assert.NoError(mt, err)

wantRawValue := bson.RawValue{
Type: iwantType,
Value: iwantBytes,
}

assert.Equal(mt, expected, actual, "expected value %v but got %v", expected, actual)
assert.EqualValues(mt, wantRawValue, gotRawValue, "expected value %v but got %v", wantRawValue, gotRawValue)
}
}

Expand Down Expand Up @@ -1636,7 +1651,11 @@ func verifyCursorResult(mt *mtest.T, cur *mongo.Cursor, result interface{}) {
}
}

func verifySingleResult(mt *mtest.T, actualResult *mongo.SingleResult, expectedResult interface{}) {
func verifySingleResult(
mt *mtest.T,
actualResult *mongo.SingleResult,
expectedResult interface{},
) {
mt.Helper()

if expectedResult == nil {
Expand Down
2 changes: 2 additions & 0 deletions internal/integration/sessions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,8 @@ func extractReturnError(returnValues []reflect.Value) error {
return converted
case *mongo.SingleResult:
return converted.Err()
case *mongo.DistinctResult:
return converted.Err()
default:
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion internal/integration/unified/admin_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func performDistinctWorkaround(ctx context.Context) error {
commandFn := func(ctx context.Context, client *mongo.Client) error {
for _, coll := range entities(ctx).collections() {
newColl := client.Database(coll.Database().Name()).Collection(coll.Name())
_, err := newColl.Distinct(ctx, "x", bson.D{})
err := newColl.Distinct(ctx, "x", bson.D{}).Err()
if err != nil {
ns := fmt.Sprintf("%s.%s", coll.Database().Name(), coll.Name())
return fmt.Errorf("error running distinct for collection %q: %w", ns, err)
Expand Down
12 changes: 7 additions & 5 deletions internal/integration/unified/collection_operation_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,15 +531,17 @@ func executeDistinct(ctx context.Context, operation *operation) (*operationResul
return nil, newMissingArgumentError("filter")
}

res, err := coll.Distinct(ctx, fieldName, filter, opts)
if err != nil {
res := coll.Distinct(ctx, fieldName, filter, opts)
if err := res.Err(); err != nil {
return newErrorResult(err), nil
}
_, rawRes, err := bson.MarshalValue(res)

arr, err := res.Raw()
if err != nil {
return nil, fmt.Errorf("error converting Distinct result to raw BSON: %w", err)
return newErrorResult(err), nil
}
return newValueResult(bson.TypeArray, rawRes, nil), nil

return newValueResult(bson.TypeArray, arr, nil), nil
}

func executeDropIndex(ctx context.Context, operation *operation) (*operationResult, error) {
Expand Down
4 changes: 2 additions & 2 deletions internal/integration/unified_spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,8 @@ func runSpecTestCase(mt *mtest.T, test *testCase, testFile testFile) {
if mtest.ClusterTopologyKind() == mtest.Sharded && test.Description == "distinct" {
err := runCommandOnAllServers(func(mongosClient *mongo.Client) error {
coll := mongosClient.Database(mt.DB.Name()).Collection(mt.Coll.Name())
_, err := coll.Distinct(context.Background(), "x", bson.D{})
return err

return coll.Distinct(context.Background(), "x", bson.D{}).Err()
})
assert.Nil(mt, err, "error running distinct against all mongoses: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion mongo/client_examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ func ExampleConnect_stableAPI() {
coll := serverAPIStrictClient.Database("db").Collection("coll")
// Fails with error: (APIStrictError) Provided apiStrict:true, but the
// command distinct is not in API Version 1
_, err = coll.Distinct(context.TODO(), "distinct", bson.D{})
err = coll.Distinct(context.TODO(), "distinct", bson.D{}).Err()
log.Println(err)

// ServerAPIOptions can be declared with a DeprecationErrors option.
Expand Down
36 changes: 15 additions & 21 deletions mongo/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -1298,16 +1298,20 @@ func (coll *Collection) EstimatedDocumentCount(ctx context.Context,
// The opts parameter can be used to specify options for the operation (see the options.DistinctOptions documentation).
//
// For more information about the command, see https://www.mongodb.com/docs/manual/reference/command/distinct/.
func (coll *Collection) Distinct(ctx context.Context, fieldName string, filter interface{},
opts ...*options.DistinctOptions) ([]interface{}, error) {
func (coll *Collection) Distinct(
ctx context.Context,
fieldName string,
filter interface{},
opts ...*options.DistinctOptions,
) *DistinctResult {

if ctx == nil {
ctx = context.Background()
}

f, err := marshal(filter, coll.bsonOpts, coll.registry)
if err != nil {
return nil, err
return &DistinctResult{err: err}
}

sess := sessionFromContext(ctx)
Expand All @@ -1319,7 +1323,7 @@ func (coll *Collection) Distinct(ctx context.Context, fieldName string, filter i

err = coll.client.validSession(sess)
if err != nil {
return nil, err
return &DistinctResult{err: err}
}

rc := coll.readConcern
Expand Down Expand Up @@ -1357,7 +1361,7 @@ func (coll *Collection) Distinct(ctx context.Context, fieldName string, filter i
if option.Comment != nil {
comment, err := marshalValue(option.Comment, coll.bsonOpts, coll.registry)
if err != nil {
return nil, err
return &DistinctResult{err: err}
}
op.Comment(comment)
}
Expand All @@ -1369,30 +1373,20 @@ func (coll *Collection) Distinct(ctx context.Context, fieldName string, filter i

err = op.Execute(ctx)
if err != nil {
return nil, replaceErrors(err)
return &DistinctResult{err: replaceErrors(err)}
}

arr, ok := op.Result().Values.ArrayOK()
if !ok {
return nil, fmt.Errorf("response field 'values' is type array, but received BSON type %s", op.Result().Values.Type)
}
err := fmt.Errorf("response field 'values' is type array, but received BSON type %s", op.Result().Values.Type)

values, err := arr.Values()
if err != nil {
return nil, err
return &DistinctResult{err: err}
}

retArray := make([]interface{}, len(values))

for i, val := range values {
raw := bson.RawValue{Type: bson.Type(val.Type), Value: val.Data}
err = raw.Unmarshal(&retArray[i])
if err != nil {
return nil, err
}
return &DistinctResult{
reg: coll.registry,
arr: bson.RawArray(arr),
}

return retArray, replaceErrors(err)
}

// mergeFindOptions combines the given FindOptions instances into a single FindOptions in a last-property-wins fashion.
Expand Down
4 changes: 2 additions & 2 deletions mongo/collection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func TestCollection(t *testing.T) {
_, err = coll.CountDocuments(bgCtx, doc)
assert.Equal(t, ErrClientDisconnected, err, "expected error %v, got %v", ErrClientDisconnected, err)

_, err = coll.Distinct(bgCtx, "x", doc)
err = coll.Distinct(bgCtx, "x", doc).Err()
assert.Equal(t, ErrClientDisconnected, err, "expected error %v, got %v", ErrClientDisconnected, err)

_, err = coll.Find(bgCtx, doc)
Expand Down Expand Up @@ -176,7 +176,7 @@ func TestCollection(t *testing.T) {
_, err = coll.CountDocuments(bgCtx, nil)
assert.Equal(t, ErrNilDocument, err, "expected error %v, got %v", ErrNilDocument, err)

_, err = coll.Distinct(bgCtx, "x", nil)
err = coll.Distinct(bgCtx, "x", nil).Err()
assert.Equal(t, ErrNilDocument, err, "expected error %v, got %v", ErrNilDocument, err)

_, err = coll.Find(bgCtx, nil)
Expand Down
7 changes: 6 additions & 1 deletion mongo/crud_examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,12 @@ func ExampleCollection_Distinct() {
// run on the server.
filter := bson.D{{"age", bson.D{{"$gt", 25}}}}
opts := options.Distinct().SetMaxTime(2 * time.Second)
values, err := coll.Distinct(context.TODO(), "name", filter, opts)
res := coll.Distinct(context.TODO(), "name", filter, opts)
if err := res.Err(); err != nil {
log.Fatal(err)
}

values, err := res.Raw()
if err != nil {
log.Fatal(err)
}
Expand Down
48 changes: 48 additions & 0 deletions mongo/results.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,51 @@ type CollectionSpecification struct {
// option is used and for MongoDB versions < 3.4.
IDIndex *IndexSpecification
}

// DistinctResult represents an array of BSON data returned from an operation.
// If the operation resulted in an error, all DistinctResult methods will return
// that error. If the operation did not return any data, all DistinctResult
// methods will return ErrNoDocuments.
type DistinctResult struct {
err error
arr bson.RawArray
reg *bson.Registry
}

// Decode will unmarshal the array represented by this DistinctResult into v. If
// there was an error from the operation that created this DistinctReuslt, that
// error will be returned. If the operation returned no array, Decode will
// return ErrNoDocuments.
//
// If the operation was successful and returned an array, Decode will return any
// errors from the unmarshalling process without any modification. If v is nil
// or is a typed nil, an error will be returned.
func (dr *DistinctResult) Decode(v any) error {
val := bson.RawValue{
Value: dr.arr,
Type: bson.TypeArray,
}

return val.UnmarshalWithRegistry(dr.reg, v)
}

// Err provides a way to check for query errors without calling Decode. Err
// returns the error, if any, that was encountered while running the operation.
// If the operation was successful but did not return any documents, Err returns
// ErrNoDocuments. If this error is not nil, this error will also be returned
// from Decode.
func (dr *DistinctResult) Err() error {
return dr.err
}

// Raw returns the document represented by this DistinctResult as a bson.Raw. If
// there was an error from the operation that created this DistinctResult, both
// the result and that error will be returned. If the operation returned no
// documents, this will return (nil, ErrNoDocuments).
func (dr *DistinctResult) Raw() (bson.RawArray, error) {
if dr.err != nil {
return nil, dr.err
}

return dr.arr, nil
}
9 changes: 8 additions & 1 deletion mongo/single_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ type SingleResult struct {
// from the one provided occurs during creation of the SingleResult, that error will be stored on the returned SingleResult.
//
// The document parameter must be a non-nil document.
func NewSingleResultFromDocument(document interface{}, err error, registry *bson.Registry) *SingleResult {
func NewSingleResultFromDocument(
document interface{},
err error,
registry *bson.Registry,
) *SingleResult {
if document == nil {
return &SingleResult{err: ErrNilDocument}
}
Expand Down Expand Up @@ -90,6 +94,7 @@ func (sr *SingleResult) Raw() (bson.Raw, error) {
if sr.err = sr.setRdrContents(); sr.err != nil {
return nil, sr.err
}

return sr.rdr, nil
}

Expand All @@ -110,7 +115,9 @@ func (sr *SingleResult) setRdrContents() error {

return ErrNoDocuments
}

sr.rdr = sr.cur.Current

return nil
}

Expand Down
Loading

0 comments on commit c481fdf

Please sign in to comment.