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

Deprecate the textpb codec #875

Merged
merged 1 commit into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 2 additions & 3 deletions docs/configuring_and_running_tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,8 @@ feature of the implementation under test.
any version of HTTP. If not configured, support is assumed for all three.
* `codecs`: This configures which codecs, or message formats, that the implementation
supports. The options are `CODEC_PROTO` (which corresponds to the sub-format "proto",
which is the Protobuf binary format), `CODEC_JSON` (sub-format "json"), and
`CODEC_TEXT` (sub-format "text", which is the Protobuf text format). If not configured,
support is assumed for "proto" and "json".
which is the Protobuf binary format) and `CODEC_JSON` (sub-format "json"). If not
configured, support is assumed for both.
* `compressions`: This configures which compression encodings are supported by the
implementation. `COMPRESSION_IDENTITY`, the "identity" encoding, means no compression.
`COMPRESSION_GZIP` indicates "gzip" compression. If not configured, support for these
Expand Down
34 changes: 34 additions & 0 deletions internal/app/connectconformance/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package connectconformance
import (
"errors"
"fmt"
"os"

"connectrpc.com/conformance/internal"
conformancev1 "connectrpc.com/conformance/internal/gen/proto/go/connectrpc/conformance/v1"
Expand Down Expand Up @@ -72,6 +73,7 @@ func parseConfig(configFileName string, data []byte) ([]configCase, error) {
if config.Features == nil {
config.Features = &conformancev1.Features{}
}
checkForDeprecations(&config)
features, err := resolveFeatures(config.Features)
if err != nil {
return nil, fmt.Errorf("%s: %w", configFileName, err)
Expand Down Expand Up @@ -318,6 +320,11 @@ func computeCasesFromFeatures(features supportedFeatures, tlsCases, tlsClientCer
}

for _, codec := range features.Codecs {
//nolint:staticcheck // staticcheck complains because this const is deprecated
if codec == conformancev1.Codec_CODEC_TEXT {
// Deprecated, ignore
continue
}
for _, compression := range features.Compressions {
for _, connectGetCase := range connectGetCases {
for _, msgRecvLimitCase := range msgRecvLimitCases {
Expand Down Expand Up @@ -415,6 +422,33 @@ func resolveCase(features supportedFeatures, unresolvedCase *conformancev1.Confi
return computeCasesFromFeatures(impliedFeatures, tlsCases, tlsClientCertCases, msgReceiveLimitCases), nil
}

func checkForDeprecations(config *conformancev1.Config) {
warn := func() {
_, _ = fmt.Fprintln(os.Stderr, "WARNING: config includes reference to CODEC_TEXT which is deprecated and will be ignored. Please remove.")
}
for _, codec := range config.GetFeatures().GetCodecs() {
//nolint:staticcheck // staticcheck complains because this const is deprecated
if codec == conformancev1.Codec_CODEC_TEXT {
warn()
return
}
}
for _, include := range config.GetIncludeCases() {
//nolint:staticcheck // staticcheck complains because this const is deprecated
if include.GetCodec() == conformancev1.Codec_CODEC_TEXT {
warn()
return
}
}
for _, exclude := range config.GetExcludeCases() {
//nolint:staticcheck // staticcheck complains because this const is deprecated
if exclude.GetCodec() == conformancev1.Codec_CODEC_TEXT {
warn()
return
}
}
}

func contains[T comparable, S ~[]T](slice S, find T) bool {
for _, elem := range slice {
if elem == find {
Expand Down
4 changes: 2 additions & 2 deletions internal/app/referenceclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ func invoke(ctx context.Context, req *conformancev1.ClientCompatRequest, referen
// this is the default, no option needed
case conformancev1.Codec_CODEC_JSON:
clientOptions = append(clientOptions, connect.WithProtoJSON())
case conformancev1.Codec_CODEC_TEXT:
clientOptions = append(clientOptions, connect.WithCodec(&internal.TextConnectCodec{}))
case conformancev1.Codec_CODEC_TEXT: //nolint:staticcheck // staticcheck complains because this const is deprecated
return nil, fmt.Errorf("%s is deprecated and should not be used", req.Codec)
default:
return nil, errors.New("a codec must be specified")
}
Expand Down
3 changes: 0 additions & 3 deletions internal/app/referenceserver/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ const (

codecProto = "proto"
codecJSON = "json"
codecText = "text"
)

// TODO - We should add a check for the Connect version header and/or query param to the reference server checks
Expand Down Expand Up @@ -162,8 +161,6 @@ func checkCodec(expected conformancev1.Codec, req *http.Request, feedback *feedb
expect = codecProto
case conformancev1.Codec_CODEC_JSON:
expect = codecJSON
case conformancev1.Codec_CODEC_TEXT:
expect = codecText
default:
feedback.Printf("invalid expected codec %d", expected)
return
Expand Down
1 change: 0 additions & 1 deletion internal/app/referenceserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ func createServer(req *conformancev1.ServerCompatRequest, listenAddr, tlsCertFil
connect.WithCompression(compression.Deflate, compression.NewDeflateDecompressor, compression.NewDeflateCompressor),
connect.WithCompression(compression.Snappy, compression.NewSnappyDecompressor, compression.NewSnappyCompressor),
connect.WithCompression(compression.Zstd, compression.NewZstdDecompressor, compression.NewZstdCompressor),
connect.WithCodec(&internal.TextConnectCodec{}),
connect.WithInterceptors(interceptors...),
}
if req.MessageReceiveLimit > 0 {
Expand Down
28 changes: 0 additions & 28 deletions internal/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"io"

"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/encoding/prototext"
"google.golang.org/protobuf/proto"
)

Expand Down Expand Up @@ -166,30 +165,3 @@ func (p *protoEncoder) Encode(msg proto.Message) error {
}
return writeDelimitedMessageRaw(p.out, data)
}

// TextConnectCodec implements the connect.Codec interface, providing the
// protobuf text format.
type TextConnectCodec struct {
prototext.MarshalOptions
prototext.UnmarshalOptions
}

func (t *TextConnectCodec) Name() string {
return "text"
}

func (t *TextConnectCodec) Marshal(a any) ([]byte, error) {
msg, ok := a.(proto.Message)
if !ok {
return nil, fmt.Errorf("message type %T does not implement proto.Message", a)
}
return t.MarshalOptions.Marshal(msg)
}

func (t *TextConnectCodec) Unmarshal(bytes []byte, a any) error {
msg, ok := a.(proto.Message)
if !ok {
return fmt.Errorf("message type %T does not implement proto.Message", a)
}
return t.UnmarshalOptions.Unmarshal(bytes, msg)
}
Loading
Loading