From ff9d11ad1a85dd18c214642f10ed078d5f90f34d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 15 Nov 2024 16:28:44 +0100 Subject: [PATCH] reuse go-micro service clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../unreleased/reuse-go-micro-service.md | 5 ++++ services/graph/pkg/service/v0/drives.go | 25 ++++++---------- services/graph/pkg/service/v0/graph_test.go | 29 +++++++++++++++---- 3 files changed, 37 insertions(+), 22 deletions(-) create mode 100644 changelog/unreleased/reuse-go-micro-service.md diff --git a/changelog/unreleased/reuse-go-micro-service.md b/changelog/unreleased/reuse-go-micro-service.md new file mode 100644 index 0000000000..6850c04ab0 --- /dev/null +++ b/changelog/unreleased/reuse-go-micro-service.md @@ -0,0 +1,5 @@ +Bugfix: Reuse go-micro service clients + +go micro clients must not be reinitialized. The internal selector will spawn a new go routine to watch for registry changes. + +https://github.com/owncloud/ocis/pull/10582 diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index 39370c96d0..25fff8b20f 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -18,6 +18,9 @@ import ( cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + revactx "github.com/cs3org/reva/v2/pkg/ctx" + "github.com/cs3org/reva/v2/pkg/storagespace" + "github.com/cs3org/reva/v2/pkg/utils" "github.com/go-chi/render" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/pkg/errors" @@ -25,12 +28,7 @@ import ( "golang.org/x/sync/errgroup" "google.golang.org/protobuf/proto" - revactx "github.com/cs3org/reva/v2/pkg/ctx" - "github.com/cs3org/reva/v2/pkg/storagespace" - "github.com/cs3org/reva/v2/pkg/utils" - "github.com/owncloud/ocis/v2/ocis-pkg/l10n" - "github.com/owncloud/ocis/v2/ocis-pkg/service/grpc" v0 "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/settings/v0" settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" "github.com/owncloud/ocis/v2/services/graph/pkg/errorcode" @@ -697,18 +695,8 @@ func (g Graph) formatDrives(ctx context.Context, baseURL *url.URL, storageSpaces // ListStorageSpacesWithFilters List Storage Spaces using filters func (g Graph) ListStorageSpacesWithFilters(ctx context.Context, filters []*storageprovider.ListStorageSpacesRequest_Filter, unrestricted bool) (*storageprovider.ListStorageSpacesResponse, error) { - gatewayClient, err := g.gatewaySelector.Next() - if err != nil { - return nil, err - } - grpcClient, err := grpc.NewClient(append(grpc.GetClientOptions(g.config.GRPCClientTLS), grpc.WithTraceProvider(g.traceProvider))...) - if err != nil { - return nil, err - } - s := settingssvc.NewPermissionService("com.owncloud.api.settings", grpcClient) - - _, err = s.GetPermissionByID(ctx, &settingssvc.GetPermissionByIDRequest{ + _, err := g.permissionsService.GetPermissionByID(ctx, &settingssvc.GetPermissionByIDRequest{ PermissionId: settingsServiceExt.ListSpacesPermission(0).Id, }) @@ -734,6 +722,11 @@ func (g Graph) ListStorageSpacesWithFilters(ctx context.Context, filters []*stor }}, Filters: filters, } + + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + return nil, err + } res, err := gatewayClient.ListStorageSpaces(ctx, lReq) return res, err } diff --git a/services/graph/pkg/service/v0/graph_test.go b/services/graph/pkg/service/v0/graph_test.go index f191ecfabb..5087de6d84 100644 --- a/services/graph/pkg/service/v0/graph_test.go +++ b/services/graph/pkg/service/v0/graph_test.go @@ -14,6 +14,12 @@ import ( userprovider "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/cs3org/reva/v2/pkg/conversions" + revactx "github.com/cs3org/reva/v2/pkg/ctx" + "github.com/cs3org/reva/v2/pkg/rgrpc/status" + "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" + "github.com/cs3org/reva/v2/pkg/utils" + cs3mocks "github.com/cs3org/reva/v2/tests/cs3mocks/mocks" "github.com/go-chi/chi/v5" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -23,12 +29,6 @@ import ( "github.com/tidwall/gjson" "google.golang.org/grpc" - "github.com/cs3org/reva/v2/pkg/conversions" - revactx "github.com/cs3org/reva/v2/pkg/ctx" - "github.com/cs3org/reva/v2/pkg/rgrpc/status" - "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" - "github.com/cs3org/reva/v2/pkg/utils" - cs3mocks "github.com/cs3org/reva/v2/tests/cs3mocks/mocks" "github.com/owncloud/ocis/v2/ocis-pkg/shared" v0 "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/settings/v0" settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" @@ -96,6 +96,13 @@ var _ = Describe("Graph", func() { Describe("Drives", func() { Describe("GetDrivesV1 and GetAllDrivesV1", func() { + BeforeEach(func() { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + // the permission is not relevant for this test + }, + }, nil) + }) It("can list an empty list of spaces", func() { gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{ Status: status.NewOK(ctx), @@ -482,6 +489,11 @@ var _ = Describe("Graph", func() { }) DescribeTable("GetDrivesV1Beta1 and GetAllDrivesV1Beta1", func(check func(gjson.Result), resourcePermissions provider.ResourcePermissions) { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + // the permission is not relevant for this test + }, + }, nil) gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Times(1).Return(&provider.ListStorageSpacesResponse{ Status: status.NewOK(ctx), StorageSpaces: []*provider.StorageSpace{ @@ -823,6 +835,11 @@ var _ = Describe("Graph", func() { Status: status.NewOK(ctx), TotalBytes: 500, }, nil) + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + // the permission is not relevant for this test + }, + }, nil) }) It("handles missing drive id", func() {