Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GODRIVER-2443 Make Distinct return a decodable struct #1603

Merged
25 changes: 15 additions & 10 deletions internal/integration/collection_test.go
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
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 {
prestonvasquez marked this conversation as resolved.
Show resolved Hide resolved
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
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
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
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
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
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
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
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
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
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
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