From 51c92839293c134173f63c337f69f9cda187edac Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Fri, 15 Sep 2017 16:27:04 +0200 Subject: Fix handling of Git object dir attributes for streaming RPCs Fixes #564 --- CHANGELOG.md | 2 ++ internal/server/server.go | 4 +++- internal/service/commit/testhelper_test.go | 10 +++++++++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b21b2c79..9f67356cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ UNRELEASED - Wait for monitor goroutine to return during supervisor shutdown https://gitlab.com/gitlab-org/gitaly/merge_requests/341 +- Fix handling of Git object dir attributes for streaming RPCs + https://gitlab.com/gitlab-org/gitaly/merge_requests/356 v0.40.0 - Use context cancellation instead of command.Close diff --git a/internal/server/server.go b/internal/server/server.go index 72c9f1904..4c82bb3e7 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -31,13 +31,15 @@ func New(rubyServer *rubyserver.Server) *grpc.Server { server := grpc.NewServer( grpc.StreamInterceptor(grpc_middleware.ChainStreamServer( - objectdirhandler.Stream, grpc_ctxtags.StreamServerInterceptor(ctxTagOpts...), grpc_prometheus.StreamServerInterceptor, grpc_logrus.StreamServerInterceptor(logrusEntry), sentryhandler.StreamLogHandler, cancelhandler.Stream, // Should be below LogHandler authStreamServerInterceptor(), + // Should be the last one, or above interceptors that + // don't replace the stream context with a derived one. + objectdirhandler.Stream, // Panic handler should remain last so that application panics will be // converted to errors and logged panichandler.StreamPanicHandler, diff --git a/internal/service/commit/testhelper_test.go b/internal/service/commit/testhelper_test.go index 7b3eaee14..a8c01cdec 100644 --- a/internal/service/commit/testhelper_test.go +++ b/internal/service/commit/testhelper_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/grpc-ecosystem/go-grpc-middleware/tags" "gitlab.com/gitlab-org/gitaly/internal/linguist" "gitlab.com/gitlab-org/gitaly/internal/middleware/objectdirhandler" "gitlab.com/gitlab-org/gitaly/internal/rubyserver" @@ -51,7 +52,14 @@ func testMain(m *testing.M) int { func startTestServices(t *testing.T) *grpc.Server { server := testhelper.NewTestGrpcServer( t, - []grpc.StreamServerInterceptor{objectdirhandler.Stream}, + []grpc.StreamServerInterceptor{ + // This interceptor only exists to make sure objectdirhandler works properly. + // If they switched positions, a test would fail. + grpc_ctxtags.StreamServerInterceptor(), + // Should be the last one, or above interceptors that + // don't replace the stream context with a derived one. + objectdirhandler.Stream, + }, []grpc.UnaryServerInterceptor{objectdirhandler.Unary}, ) -- cgit v1.2.3