Skip to content

Commit bb5ce2f

Browse files
committed
addressed comments
1 parent 1364348 commit bb5ce2f

8 files changed

Lines changed: 209 additions & 278 deletions

internal/xds/httpfilter/httpfilter.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,13 @@ type ClientFilter interface {
135135
type ServerInterceptor interface {
136136
// InterceptRPC intercepts an incoming RPC on the server side.
137137
//
138-
// On success, implementations must return either the original ServerStream or
139-
// a wrapped ServerStream, with a nil error.
138+
// On success, implementations must return either the original ServerStream
139+
// or a wrapped ServerStream, with a nil error.
140+
//
141+
// Returning a non-nil error will terminate the RPC with that error.
142+
// Implementations are expected to return an error created using the status
143+
// package; otherwise, the RPC will fail with an UNKNOWN status code.
140144
//
141-
// Returning a non-nil error will terminate the RPC with that status error.
142145
// Implementations should never return (nil, nil).
143146
InterceptRPC(ss grpc.ServerStream) (grpc.ServerStream, error)
144147

test/server_test.go

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -486,27 +486,39 @@ func (s) TestInterceptorSegregation(t *testing.T) {
486486
}
487487
}
488488

489+
type wrappedTestStreamKey struct{}
490+
489491
type wrappedTestStream struct {
490492
grpc.ServerStream
491493
}
492494

495+
func (w *wrappedTestStream) Context() context.Context {
496+
return context.WithValue(w.ServerStream.Context(), wrappedTestStreamKey{}, w)
497+
}
498+
493499
// Test verifies that an internal xDS filter wrapper option configured on the
494500
// server gets invoked and can wrap the ServerStream for both Unary and
495501
// Streaming RPCs.
496502
func (s) TestXDSFilterWrapperOption(t *testing.T) {
497-
var wrapperCalled atomic.Bool
503+
var wrapperCallCount atomic.Int32
498504
wrapper := func(ss grpc.ServerStream) (grpc.ServerStream, error) {
499-
wrapperCalled.Store(true)
505+
wrapperCallCount.Add(1)
500506
return &wrappedTestStream{ServerStream: ss}, nil
501507
}
502508

503509
opt := internal.XDSFilterWrapperOption.(func(func(grpc.ServerStream) (grpc.ServerStream, error)) grpc.ServerOption)(wrapper)
504510

505511
ss := &stubserver.StubServer{
506-
EmptyCallF: func(context.Context, *testpb.Empty) (*testpb.Empty, error) {
512+
EmptyCallF: func(ctx context.Context, _ *testpb.Empty) (*testpb.Empty, error) {
513+
if _, ok := ctx.Value(wrappedTestStreamKey{}).(*wrappedTestStream); !ok {
514+
return nil, status.Errorf(codes.Internal, "context value is %T, want *wrappedTestStream", ctx.Value(wrappedTestStreamKey{}))
515+
}
507516
return &testpb.Empty{}, nil
508517
},
509-
FullDuplexCallF: func(testgrpc.TestService_FullDuplexCallServer) error {
518+
FullDuplexCallF: func(stream testgrpc.TestService_FullDuplexCallServer) error {
519+
if _, ok := stream.Context().Value(wrappedTestStreamKey{}).(*wrappedTestStream); !ok {
520+
return status.Errorf(codes.Internal, "context value is %T, want *wrappedTestStream", stream.Context().Value(wrappedTestStreamKey{}))
521+
}
510522
return nil
511523
},
512524
}
@@ -520,20 +532,19 @@ func (s) TestXDSFilterWrapperOption(t *testing.T) {
520532
if _, err := ss.Client.EmptyCall(ctx, &testpb.Empty{}); err != nil {
521533
t.Fatalf("EmptyCall failed: %v", err)
522534
}
523-
if !wrapperCalled.Load() {
524-
t.Fatal("XDSFilterWrapperOption callback was not called for Unary RPC")
535+
if got := wrapperCallCount.Load(); got != 1 {
536+
t.Fatalf("XDSFilterWrapperOption callback call count for Unary RPC got %d, want 1", got)
525537
}
526538

527-
wrapperCalled.Store(false)
528539
stream, err := ss.Client.FullDuplexCall(ctx)
529540
if err != nil {
530541
t.Fatalf("FullDuplexCall failed: %v", err)
531542
}
532543
if _, err = stream.Recv(); err != io.EOF {
533544
t.Fatalf("Recv failed: %v", err)
534545
}
535-
if !wrapperCalled.Load() {
536-
t.Fatal("XDSFilterWrapperOption callback was not called for Streaming RPC")
546+
if got := wrapperCallCount.Load(); got != 2 {
547+
t.Fatalf("XDSFilterWrapperOption callback call count for Streaming RPC got %d, want 2", got)
537548
}
538549
}
539550

0 commit comments

Comments
 (0)