Fix CDC read error (#96)

CDCEvent reads the _id field into a string. By default, the Mongo client v2 will not convert the ObjectID to a string, so we get an error at read time. To fix this, we now set BSONOptions on the collection to allow this deserialization. This wasn't caught in the previous tests because the test code explicitly set the _id field to a string when creating test events, so the test events deserialized without error. I've also updated the TestWriteSuccess test to use a Mongo generated _id which reproduces the error.

Alternatively I considered changing CDCEvent.ID to a bson.ObjectID type, but since this struct is used in the REST client library I do not want to leak that Mongo type and dependency into the client. I also considered creating an internal cdcEvent struct identical to CDCEvent but with the bson.ObjectID type, then have the server deserialize into that struct and copy to the CDCEvent struct, but that seemed heavy and unnecessary.

Author: jemiahw

cdc/store_test.go

@@ -113,8 +113,10 @@ func TestRead(t *testing.T) {
 func TestWriteSuccess(t *testing.T) {
 	cdcs := setup(t, "", cdc.NoRetryPolicy)
 
+	// Note we don't explicitly set ID here, so that MongoDB will generate an _id for us.
+	// This is the same behavior as the actual store implementation, which does not set the _id and
+	// allows MongoDB to generate it.
 	event := etre.CDCEvent{
-		Id:         "abc",
 		Ts:         54,
 		Op:         "i",
 		Caller:     "mike",
@@ -140,6 +142,19 @@ func TestWriteSuccess(t *testing.T) {
 	actualEvents, err := cdcs.Read(filter)
 	require.NoError(t, err)
 	assert.Len(t, actualEvents, 1)
+
+	// MongoDB auto-generates the _id column since we didn't provide one in `event` above.
+	// This is the same behavior as the actual store implementation, which does not set the _id and
+	// allows MongoDB to generate it.
+	//
+	// Since we don't know what the generated _id will be, we can't compare the entire event directly.
+	// Instead, verify that the _id is a valid ObjectID and then clear it so we can compare the rest
+	// of the event we read (in actualEvents[0]) with the original `event` above.
+	_, err = bson.ObjectIDFromHex(actualEvents[0].Id)
+	require.NoError(t, err)
+	actualEvents[0].Id = ""
+
+	// Compare the rest of the event
 	assert.Equal(t, event, actualEvents[0])
 }
 

server/server.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"go.mongodb.org/mongo-driver/v2/mongo"
+	"go.mongodb.org/mongo-driver/v2/mongo/options"
 
 	"github.com/square/etre"
 	"github.com/square/etre/api"
@@ -64,7 +65,8 @@ func (s *Server) Boot(configFile string) error {
 			return fmt.Errorf("cannot connect to CDC datasource: %s", err)
 		}
 		s.cdcDbClient = cdcClient
-		cdcColl := cdcClient.Database(cfg.Datasource.Database).Collection(config.CDC_COLLECTION)
+		cdcOpts := options.Collection().SetBSONOptions(&options.BSONOptions{ObjectIDAsHexString: true}) // Because etre.CDCEvent has string _id, not bson.ObjectID
+		cdcColl := cdcClient.Database(cfg.Datasource.Database).Collection(config.CDC_COLLECTION, cdcOpts)
 
 		// Store
 		wrp := cdc.RetryPolicy{

test/db.go

@@ -24,7 +24,12 @@ func DbCollections(entityTypes []string) (*mongo.Client, map[string]*mongo.Colle
 	}
 	coll := map[string]*mongo.Collection{}
 	for _, t := range entityTypes {
-		coll[t] = client.Database(database).Collection(t)
+		if t == "cdc" {
+			cdcOpts := options.Collection().SetBSONOptions(&options.BSONOptions{ObjectIDAsHexString: true}) // Because etre.CDCEvent has string _id, not bson.ObjectID
+			coll[t] = client.Database(database).Collection(t, cdcOpts)
+		} else {
+			coll[t] = client.Database(database).Collection(t)
+		}
 	}
 	return client, coll, nil
 }