Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .evergreen/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ functions:
params:
binary: bash
env:
GO_BUILD_TAGS: cse
GO_BUILD_TAGS: "cse,mongointernal"
include_expansions_in_env: ["TOPOLOGY", "AUTH", "SSL", "SKIP_CSOT_TESTS", "MONGODB_URI", "CRYPT_SHARED_LIB_PATH", "SKIP_CRYPT_SHARED_LIB", "RACE", "MONGO_GO_DRIVER_COMPRESSOR", "REQUIRE_API_VERSION", "LOAD_BALANCER"]
args: [*task-runner, setup-test]
- command: subprocess.exec
Expand Down
73 changes: 73 additions & 0 deletions internal/integration/mongointernal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright (C) MongoDB, Inc. 2025-present.
//
// Licensed under the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License. You may obtain
// a copy of the License at http://www.apache.org/licenses/LICENSE-2.0

//go:build mongointernal

package integration

import (
"context"
"testing"

"go.mongodb.org/mongo-driver/v2/bson"
"go.mongodb.org/mongo-driver/v2/internal/assert"
"go.mongodb.org/mongo-driver/v2/internal/integration/mtest"
"go.mongodb.org/mongo-driver/v2/internal/require"
"go.mongodb.org/mongo-driver/v2/mongo"
"go.mongodb.org/mongo-driver/v2/x/bsonx/bsoncore"
)

func TestNewSessionWithID(t *testing.T) {
mt := mtest.New(t)

mt.Run("can be used to pass a specific session ID to CRUD commands", func(mt *mtest.T) {
mt.Parallel()

// Create a session ID document, which is a BSON document with field
// "id" containing a 16-byte UUID (binary subtype 4).
sessionID := bson.Raw(bsoncore.NewDocumentBuilder().
AppendBinary("id", 4, []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}).
Build())

sess := mongo.NewSessionWithID(mt.Client, sessionID)

ctx := mongo.NewSessionContext(context.Background(), sess)
_, err := mt.Coll.InsertOne(ctx, bson.D{{"foo", "bar"}})
require.NoError(mt, err)

evt := mt.GetStartedEvent()
val, err := evt.Command.LookupErr("lsid")
require.NoError(mt, err, "lsid should be present in the command document")

doc, ok := val.DocumentOK()
require.True(mt, ok, "lsid should be a document")

assert.Equal(mt, sessionID, doc)
})

mt.Run("EndSession panics", func(mt *mtest.T) {
mt.Parallel()

sessionID := bson.Raw(bsoncore.NewDocumentBuilder().
AppendBinary("id", 4, []byte{}).
Copy link
Member

@prestonvasquez prestonvasquez Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Suggest using a valid 16-byte UUID to ensure the panic is due to the session being unpooled and not an invalid LSID.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Build())
sess := mongo.NewSessionWithID(mt.Client, sessionID)

// Use a defer-recover block to catch the expected panic and assert that
// the recovered error is not nil.
defer func() {
err := recover()
assert.NotNil(mt, err, "expected panic error to not be nil")
}()

sess.EndSession(context.Background())

// We expect that calling EndSession on a Session returned by
// NewSessionWithID panics. This code will only be reached if EndSession
// doesn't panic.
t.Errorf("expected EndSession to panic")
})
}
41 changes: 41 additions & 0 deletions mongo/mongointernal.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (C) MongoDB, Inc. 2025-present.
//
// Licensed under the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License. You may obtain
// a copy of the License at http://www.apache.org/licenses/LICENSE-2.0

//go:build mongointernal
Copy link
Member

@prestonvasquez prestonvasquez Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[non-blocking] Should we name this mongointernal_lsid to keep it separate from any future code we may want to gate behind a mongointernal build tag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned too many build tags will become a maintenance headache, both for us and for the internal teams who use the Go Driver. I'm assuming that an app that uses one internal-only API may need to use many internal-only APIs, and keeping track of a list of build tags may become onerous.

Is there a scenario you're thinking of where it's better to have more granular build tags?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a scenario you're thinking of where it's better to have more granular build tags?

No, and the more I think about it it seems unlikely that we will need to use mongointernal often.


package mongo

import (
"time"

"go.mongodb.org/mongo-driver/v2/bson"
"go.mongodb.org/mongo-driver/v2/x/bsonx/bsoncore"
"go.mongodb.org/mongo-driver/v2/x/mongo/driver/session"
)

// NewSessionWithID returns a Session with the given sessionID document. The
// sessionID is a BSON document with key "id" containing a 16-byte UUID (binary
// subtype 4).
//
// Sessions returned by NewSessionWithID are never added to the driver's session
// pool. Calling EndSession on a Session returned by NewSessionWithID will
Copy link
Member

@prestonvasquez prestonvasquez Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[non-blocking] We should include SetServer as API that will panic. I'm not sure why anyone would use it and AFAIK it's only use case happens for implicit sessions, but it's still possible:

sess.Client().SetSrerver() // will panic 

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added note that ClientSession().SetServer() will panic and a test to confirm it does.

// panic.
//
// NewSessionWithID is intended only for internal use and may be changed or
// removed at any time.
func NewSessionWithID(client *Client, sessionID bson.Raw) *Session {
Copy link
Member

@prestonvasquez prestonvasquez Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Suggest renaming this function to NewSessionWithLSID

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed.

return &Session{
clientSession: &session.Client{
Server: &session.Server{
SessionID: bsoncore.Document(sessionID),
LastUsed: time.Now(),
},
ClientID: client.id,
},
client: client,
deployment: client.deployment,
}
}
Loading