control: use generics for response wrappers #290

Merged
fyrchik merged 1 commit from fyrchik/frostfs-node:control-generics into master 2023-04-28 13:59:44 +00:00
2 changed files with 31 additions and 187 deletions

View file

@ -14,171 +14,26 @@ func (w *requestWrapper) ToGRPCMessage() grpc.Message {
return w.m return w.m
} }
type healthCheckResponseWrapper struct { type responseWrapper[T grpc.Message] struct {
ale64bit marked this conversation as resolved
Review

this is kinda duped now:

type responseWrapper[M grpc.Message] struct {


let's move it to a common package and reuse it?

this is kinda duped now: https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/ff255212046f508faa637b04d61418bc5bb39fd4/pkg/services/control/ir/convert.go#L17 let's move it to a common package and reuse it?
Review

My rule of thumb is to abstract something when we have 3 occurences, we have 2 currently :)
The helper looks simple enough to tolerate some amount of duplication.

My rule of thumb is to abstract something when we have 3 occurences, we have 2 currently :) The helper looks simple enough to tolerate some amount of duplication.
Review

As you wish.

IMHO, abstracting it is trivial (move it to a separate package) and it's reasonable to expect that the abstraction will be used more than two places, since it serves to write many wrappers of the same type which are prevalent in the codebase. What's the benefit of tolerating the duplication anyway?

Could you elaborate how you came to the constant 3 in your rule of thumb? I'm curious about the rationale behind it.

As you wish. IMHO, abstracting it is trivial (move it to a separate package) and it's reasonable to expect that the abstraction will be used more than two places, since it serves to write many wrappers of the same type which are prevalent in the codebase. What's the benefit of tolerating the duplication anyway? Could you elaborate how you came to the constant 3 in your rule of thumb? I'm curious about the rationale behind it.
Review

We will simplify the api-go itself in the near future, as @dstepanov-yadro noticed, these wrappers look like a kludge.

We will simplify the api-go itself in the near future, as @dstepanov-yadro noticed, these wrappers look like a kludge.
m *HealthCheckResponse message *T
} }
func (w *healthCheckResponseWrapper) ToGRPCMessage() grpc.Message { func newResponseWrapper[T grpc.Message]() *responseWrapper[T] {

Maybe replace any -> grpc.Message? It looks strange if you don't look at grpc.Message definition.

Maybe replace any -> grpc.Message? It looks strange if you don't look at `grpc.Message` definition.

Fixed

Fixed
return w.m return &responseWrapper[T]{
message: new(T),
}
} }
func (w *healthCheckResponseWrapper) FromGRPCMessage(m grpc.Message) error { func (w *responseWrapper[T]) ToGRPCMessage() grpc.Message {
var ok bool return w.message
}
w.m, ok = m.(*HealthCheckResponse) func (w *responseWrapper[T]) FromGRPCMessage(m grpc.Message) error {
response, ok := m.(*T)
if !ok { if !ok {
return message.NewUnexpectedMessageType(m, w.m) return message.NewUnexpectedMessageType(m, w.message)
} }
return nil w.message = response
}
type setNetmapStatusResponseWrapper struct {
message.Message
m *SetNetmapStatusResponse
}
func (w *setNetmapStatusResponseWrapper) ToGRPCMessage() grpc.Message {
return w.m
}
func (w *setNetmapStatusResponseWrapper) FromGRPCMessage(m grpc.Message) error {
var ok bool
w.m, ok = m.(*SetNetmapStatusResponse)
if !ok {
return message.NewUnexpectedMessageType(m, w.m)
}
return nil
}
type dropObjectsResponseWrapper struct {
message.Message
m *DropObjectsResponse
}
func (w *dropObjectsResponseWrapper) ToGRPCMessage() grpc.Message {
return w.m
}
func (w *dropObjectsResponseWrapper) FromGRPCMessage(m grpc.Message) error {
var ok bool
w.m, ok = m.(*DropObjectsResponse)
if !ok {
return message.NewUnexpectedMessageType(m, w.m)
}
return nil
}
type listShardsResponseWrapper struct {
m *ListShardsResponse
}
func (w *listShardsResponseWrapper) ToGRPCMessage() grpc.Message {
return w.m
}
func (w *listShardsResponseWrapper) FromGRPCMessage(m grpc.Message) error {
var ok bool
w.m, ok = m.(*ListShardsResponse)
if !ok {
return message.NewUnexpectedMessageType(m, w.m)
}
return nil
}
type setShardModeResponseWrapper struct {
m *SetShardModeResponse
}
func (w *setShardModeResponseWrapper) ToGRPCMessage() grpc.Message {
return w.m
}
func (w *setShardModeResponseWrapper) FromGRPCMessage(m grpc.Message) error {
var ok bool
w.m, ok = m.(*SetShardModeResponse)
if !ok {
return message.NewUnexpectedMessageType(m, w.m)
}
return nil
}
type synchronizeTreeResponseWrapper struct {
*SynchronizeTreeResponse
}
func (w *synchronizeTreeResponseWrapper) ToGRPCMessage() grpc.Message {
return w.SynchronizeTreeResponse
}
func (w *synchronizeTreeResponseWrapper) FromGRPCMessage(m grpc.Message) error {
r, ok := m.(*SynchronizeTreeResponse)
if !ok {
return message.NewUnexpectedMessageType(m, (*SynchronizeTreeResponse)(nil))
}
w.SynchronizeTreeResponse = r
return nil
}
type evacuateShardResponseWrapper struct {
*EvacuateShardResponse
}
func (w *evacuateShardResponseWrapper) ToGRPCMessage() grpc.Message {
return w.EvacuateShardResponse
}
func (w *evacuateShardResponseWrapper) FromGRPCMessage(m grpc.Message) error {
r, ok := m.(*EvacuateShardResponse)
if !ok {
return message.NewUnexpectedMessageType(m, (*EvacuateShardResponse)(nil))
}
w.EvacuateShardResponse = r
return nil
}
type flushCacheResponseWrapper struct {
*FlushCacheResponse
}
func (w *flushCacheResponseWrapper) ToGRPCMessage() grpc.Message {
return w.FlushCacheResponse
}
func (w *flushCacheResponseWrapper) FromGRPCMessage(m grpc.Message) error {
r, ok := m.(*FlushCacheResponse)
if !ok {
return message.NewUnexpectedMessageType(m, (*FlushCacheResponse)(nil))
}
w.FlushCacheResponse = r
return nil
}
type doctorResponseWrapper struct {
*DoctorResponse
}
func (w *doctorResponseWrapper) ToGRPCMessage() grpc.Message {
return w.DoctorResponse
}
func (w *doctorResponseWrapper) FromGRPCMessage(m grpc.Message) error {
r, ok := m.(*DoctorResponse)
if !ok {
return message.NewUnexpectedMessageType(m, (*DoctorResponse)(nil))
}
w.DoctorResponse = r
return nil return nil
} }

View file

@ -25,10 +25,7 @@ func HealthCheck(
req *HealthCheckRequest, req *HealthCheckRequest,
opts ...client.CallOption, opts ...client.CallOption,
) (*HealthCheckResponse, error) { ) (*HealthCheckResponse, error) {
wResp := &healthCheckResponseWrapper{ wResp := newResponseWrapper[HealthCheckResponse]()
m: new(HealthCheckResponse),
}
wReq := &requestWrapper{ wReq := &requestWrapper{
m: req, m: req,
} }
@ -38,7 +35,7 @@ func HealthCheck(
return nil, err return nil, err
} }
return wResp.m, nil return wResp.message, nil
} }
// SetNetmapStatus executes ControlService.SetNetmapStatus RPC. // SetNetmapStatus executes ControlService.SetNetmapStatus RPC.
@ -47,9 +44,7 @@ func SetNetmapStatus(
req *SetNetmapStatusRequest, req *SetNetmapStatusRequest,
opts ...client.CallOption, opts ...client.CallOption,
) (*SetNetmapStatusResponse, error) { ) (*SetNetmapStatusResponse, error) {
wResp := &setNetmapStatusResponseWrapper{ wResp := newResponseWrapper[SetNetmapStatusResponse]()
m: new(SetNetmapStatusResponse),
}
wReq := &requestWrapper{ wReq := &requestWrapper{
m: req, m: req,
@ -60,7 +55,7 @@ func SetNetmapStatus(
return nil, err return nil, err
} }
return wResp.m, nil return wResp.message, nil
} }
// DropObjects executes ControlService.DropObjects RPC. // DropObjects executes ControlService.DropObjects RPC.
@ -69,9 +64,7 @@ func DropObjects(
req *DropObjectsRequest, req *DropObjectsRequest,
opts ...client.CallOption, opts ...client.CallOption,
) (*DropObjectsResponse, error) { ) (*DropObjectsResponse, error) {
wResp := &dropObjectsResponseWrapper{ wResp := newResponseWrapper[DropObjectsResponse]()
m: new(DropObjectsResponse),
}
wReq := &requestWrapper{ wReq := &requestWrapper{
m: req, m: req,
@ -81,7 +74,7 @@ func DropObjects(
return nil, err return nil, err
} }
return wResp.m, nil return wResp.message, nil
} }
// ListShards executes ControlService.ListShards RPC. // ListShards executes ControlService.ListShards RPC.
@ -90,9 +83,7 @@ func ListShards(
req *ListShardsRequest, req *ListShardsRequest,
opts ...client.CallOption, opts ...client.CallOption,
) (*ListShardsResponse, error) { ) (*ListShardsResponse, error) {
wResp := &listShardsResponseWrapper{ wResp := newResponseWrapper[ListShardsResponse]()
m: new(ListShardsResponse),
}
wReq := &requestWrapper{ wReq := &requestWrapper{
m: req, m: req,
@ -102,7 +93,7 @@ func ListShards(
return nil, err return nil, err
} }
return wResp.m, nil return wResp.message, nil
} }
// SetShardMode executes ControlService.SetShardMode RPC. // SetShardMode executes ControlService.SetShardMode RPC.
@ -111,9 +102,7 @@ func SetShardMode(
req *SetShardModeRequest, req *SetShardModeRequest,
opts ...client.CallOption, opts ...client.CallOption,
) (*SetShardModeResponse, error) { ) (*SetShardModeResponse, error) {
wResp := &setShardModeResponseWrapper{ wResp := newResponseWrapper[SetShardModeResponse]()
m: new(SetShardModeResponse),
}
wReq := &requestWrapper{ wReq := &requestWrapper{
m: req, m: req,
@ -123,12 +112,12 @@ func SetShardMode(
return nil, err return nil, err
} }
return wResp.m, nil return wResp.message, nil
} }
// SynchronizeTree executes ControlService.SynchronizeTree RPC. // SynchronizeTree executes ControlService.SynchronizeTree RPC.
func SynchronizeTree(cli *client.Client, req *SynchronizeTreeRequest, opts ...client.CallOption) (*SynchronizeTreeResponse, error) { func SynchronizeTree(cli *client.Client, req *SynchronizeTreeRequest, opts ...client.CallOption) (*SynchronizeTreeResponse, error) {
wResp := &synchronizeTreeResponseWrapper{new(SynchronizeTreeResponse)} wResp := newResponseWrapper[SynchronizeTreeResponse]()
wReq := &requestWrapper{m: req} wReq := &requestWrapper{m: req}
err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcSynchronizeTree), wReq, wResp, opts...) err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcSynchronizeTree), wReq, wResp, opts...)
@ -136,12 +125,12 @@ func SynchronizeTree(cli *client.Client, req *SynchronizeTreeRequest, opts ...cl
return nil, err return nil, err
} }
return wResp.SynchronizeTreeResponse, nil return wResp.message, nil
} }
// EvacuateShard executes ControlService.EvacuateShard RPC. // EvacuateShard executes ControlService.EvacuateShard RPC.
func EvacuateShard(cli *client.Client, req *EvacuateShardRequest, opts ...client.CallOption) (*EvacuateShardResponse, error) { func EvacuateShard(cli *client.Client, req *EvacuateShardRequest, opts ...client.CallOption) (*EvacuateShardResponse, error) {
wResp := &evacuateShardResponseWrapper{new(EvacuateShardResponse)} wResp := newResponseWrapper[EvacuateShardResponse]()
wReq := &requestWrapper{m: req} wReq := &requestWrapper{m: req}
err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcEvacuateShard), wReq, wResp, opts...) err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcEvacuateShard), wReq, wResp, opts...)
@ -149,12 +138,12 @@ func EvacuateShard(cli *client.Client, req *EvacuateShardRequest, opts ...client
return nil, err return nil, err
} }
return wResp.EvacuateShardResponse, nil return wResp.message, nil
} }
// FlushCache executes ControlService.FlushCache RPC. // FlushCache executes ControlService.FlushCache RPC.
func FlushCache(cli *client.Client, req *FlushCacheRequest, opts ...client.CallOption) (*FlushCacheResponse, error) { func FlushCache(cli *client.Client, req *FlushCacheRequest, opts ...client.CallOption) (*FlushCacheResponse, error) {
wResp := &flushCacheResponseWrapper{new(FlushCacheResponse)} wResp := newResponseWrapper[FlushCacheResponse]()
wReq := &requestWrapper{m: req} wReq := &requestWrapper{m: req}
err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcFlushCache), wReq, wResp, opts...) err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcFlushCache), wReq, wResp, opts...)
@ -162,12 +151,12 @@ func FlushCache(cli *client.Client, req *FlushCacheRequest, opts ...client.CallO
return nil, err return nil, err
} }
return wResp.FlushCacheResponse, nil return wResp.message, nil
} }
// Doctor executes ControlService.Doctor RPC. // Doctor executes ControlService.Doctor RPC.
func Doctor(cli *client.Client, req *DoctorRequest, opts ...client.CallOption) (*DoctorResponse, error) { func Doctor(cli *client.Client, req *DoctorRequest, opts ...client.CallOption) (*DoctorResponse, error) {
wResp := &doctorResponseWrapper{new(DoctorResponse)} wResp := newResponseWrapper[DoctorResponse]()
wReq := &requestWrapper{m: req} wReq := &requestWrapper{m: req}
err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcDoctor), wReq, wResp, opts...) err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcDoctor), wReq, wResp, opts...)
@ -175,5 +164,5 @@ func Doctor(cli *client.Client, req *DoctorRequest, opts ...client.CallOption) (
return nil, err return nil, err
} }
return wResp.DoctorResponse, nil return wResp.message, nil
} }