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-2706 Deprecate all code in the bsonoptions package. #1269

Merged

Conversation

matthewdale
Copy link
Collaborator

@matthewdale matthewdale commented May 17, 2023

GODRIVER-2706

Summary

  • Deprecate all types and methods in the bsonoptions package with suggested alternatives.
  • Add the Decoder.UseLocalTimeZone option and deprecate TimeCodec.UseLocalTimeZone and the corresponding bsonoptions options. Note this should have been done with #1229, but I missed it.
  • Deprecate StringCodec.DecodeObjectIDAsHex. Note this should have been done with #1229, but I missed it.

Background & Motivation

Currently the bsonoptions package is used exclusively to define configuration options for types and functions in the bsoncodec package. Those options structs are almost exact duplicates of most of the *Codec structs, which are all deprecated now as well.

@matthewdale matthewdale marked this pull request as ready for review May 17, 2023 20:43
Copy link
Collaborator

@prestonvasquez prestonvasquez left a comment

Choose a reason for hiding this comment

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

LGTM

bson/bsoncodec/time_codec.go Show resolved Hide resolved
@matthewdale matthewdale merged commit fc74840 into mongodb:master May 24, 2023
17 of 19 checks passed
@quipo
Copy link

quipo commented Jun 27, 2023

Can you also update the examples here? https://github.com/mongodb/mongo-go-driver/blob/master/bson/mgocompat/registry.go

@quipo
Copy link

quipo commented Jun 27, 2023

the deprecation comment isn't helpful, as it suggest to register StructCodec, which is itself deprecated:

// Deprecated: Use [go.mongodb.org/mongo-driver/bson.NewRegistry] to get a registry with the
// StructCodec registered.
type StructCodec struct {

@matthewdale
Copy link
Collaborator Author

@quipo The deprecation note on StructCodec is intended to communicate that creating and using the StructCodec directly will not be supported in Go Driver 2.0. To access the encode/decode behavior of StructCodec, you can create a new registry with bson.NewRegistry and then use LookupEncoder or LookupDecoder with your struct type (check out an example here). Does that handle your use case? If not, can you provide more details about your use case for StructCodec?

I'm not sure what you mean about updating examples in mgocompat/registry.go. Are you looking for reference for how to build an mgo-compatible registry without the deprecated types?

@quipo
Copy link

quipo commented Jun 29, 2023

Hi @matthewdale, thanks for your kind reply.

Let me expand on what I was trying to say...

  1. The way I read it, the deprecation message for the StructCodec struct suggests registering StructCodec with the registry, rather than initialising bsonoptions.StructCodec() by itself. But StructCodec being deprecated, I don't know what to pass to the Registry:
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.NewRegistry] to get a registry with the
// StructCodec registered.
type StructCodec struct {

So to me the comment isn't useful in understanding what the suggested migration is.
I'll play with the LookupEncoder alternative you mentioned.

  1. the mgocompat/registry.go example showed how to configure the codec (eg setting SetDecodeZeroStruct, SetEncodeOmitDefaultStruct) separately from the specific encoder/decoder instance utilisation, so it could be applied to all encoder/decoder instances. The suggested migration to configuring the same settings in the encoder/decoder has the downside of requiring the buffer itself, which is something I'd expect to do each time I need to encode/decode something (rather than in the general configuration of the *options.ClientOptions instance). Does it make sense?

Here's what I had:

import (
	"reflect"

	"go.mongodb.org/mongo-driver/bson"
	"go.mongodb.org/mongo-driver/bson/bsoncodec"
	"go.mongodb.org/mongo-driver/bson/bsonoptions"
	"go.mongodb.org/mongo-driver/mongo/options"
)

func NewMongoClientOptions(mongoDBURI string) (*options.ClientOptions, error) {
	// options copied from
	// https://github.com/mongodb/mongo-go-driver/blob/master/bson/mgocompat/registry.go#L50-L55
	opts := bsonoptions.StructCodec().
		SetDecodeZeroStruct(true).
		SetEncodeOmitDefaultStruct(true).
		SetOverwriteDuplicatedInlinedFields(false).
		SetAllowUnexportedFields(true)

	structCodec, err := bsoncodec.NewStructCodec(bsoncodec.JSONFallbackStructTagParser, opts)
	if err != nil {
		return nil, err
	}

	// use default registry builder settings except for
	// default encoder and decoder
	registry := bson.NewRegistryBuilder().
		RegisterDefaultDecoder(reflect.Struct, structCodec).
		RegisterDefaultEncoder(reflect.Struct, structCodec).
		Build()

	return options.Client().ApplyURI(mongoDBURI).SetRegistry(registry), nil
}

Thanks!

@matthewdale
Copy link
Collaborator Author

@quipo Thank you for the example, that clarifies the question a lot! Here's the new way to configure a Client with the same behavior:

import "go.mongodb.org/mongo-driver/mongo/options"

func NewMongoClientOptions(mongoDBURI string) (*options.ClientOptions, error) {
	bsonOpts := &options.BSONOptions{
		ZeroStructs:             true,
		OmitZeroStruct:          true,
		ErrorOnInlineDuplicates: true,
	}

	return options.Client().ApplyURI(mongoDBURI).SetBSONOptions(bsonOpts), nil
}

Note that AllowUnexportedFields is not supported in the new API because of inconsistent behavior accessing unexported struct fields with the reflect package across different Go versions.

I agree the deprecation notice should more clearly identify the replacement functionality. I've created GODRIVER-2886 to track that improvement.

@quipo
Copy link

quipo commented Jul 4, 2023

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants