Unify parameters in object services #495

Merged
fyrchik merged 10 commits from dstepanov-yadro/frostfs-node:refactor/getsvc_unify_params into master 2023-07-12 07:42:12 +00:00
17 changed files with 179 additions and 497 deletions

View file

@ -313,48 +313,40 @@ func createPutSvc(c *cfg, keyStorage *util.KeyStorage) *putsvc.Service {
} }
return putsvc.NewService( return putsvc.NewService(
putsvc.WithKeyStorage(keyStorage), keyStorage,
putsvc.WithClientConstructor(c.putClientCache), c.putClientCache,
putsvc.WithMaxSizeSource(newCachedMaxObjectSizeSource(c)), newCachedMaxObjectSizeSource(c),
putsvc.WithObjectStorage(os), os,
putsvc.WithContainerSource(c.cfgObject.cnrSource), c.cfgObject.cnrSource,
putsvc.WithNetworkMapSource(c.netMapSource), c.netMapSource,
putsvc.WithNetmapKeys(c), c,
putsvc.WithNetworkState(c.cfgNetmap.state), c.cfgNetmap.state,
putsvc.WithWorkerPools(c.cfgObject.pool.putRemote, c.cfgObject.pool.putLocal), putsvc.WithWorkerPools(c.cfgObject.pool.putRemote, c.cfgObject.pool.putLocal),
putsvc.WithLogger(c.log), putsvc.WithLogger(c.log),
) )
} }
func createPutSvcV2(sPut *putsvc.Service, keyStorage *util.KeyStorage) *putsvcV2.Service { func createPutSvcV2(sPut *putsvc.Service, keyStorage *util.KeyStorage) *putsvcV2.Service {
return putsvcV2.NewService( return putsvcV2.NewService(sPut, keyStorage)
putsvcV2.WithInternalService(sPut),
putsvcV2.WithKeyStorage(keyStorage),
)
} }
func createSearchSvc(c *cfg, keyStorage *util.KeyStorage, traverseGen *util.TraverserGenerator, coreConstructor *cache.ClientCache) *searchsvc.Service { func createSearchSvc(c *cfg, keyStorage *util.KeyStorage, traverseGen *util.TraverserGenerator, coreConstructor *cache.ClientCache) *searchsvc.Service {
ls := c.cfgObject.cfgLocalStorage.localStorage ls := c.cfgObject.cfgLocalStorage.localStorage
return searchsvc.New( return searchsvc.New(
searchsvc.WithLogger(c.log), ls,
searchsvc.WithLocalStorageEngine(ls), coreConstructor,
searchsvc.WithClientConstructor(coreConstructor),
searchsvc.WithTraverserGenerator(
traverseGen.WithTraverseOptions( traverseGen.WithTraverseOptions(
placement.WithoutSuccessTracking(), placement.WithoutSuccessTracking(),
), ),
), c.netMapSource,
searchsvc.WithNetMapSource(c.netMapSource), keyStorage,
searchsvc.WithKeyStorage(keyStorage), searchsvc.WithLogger(c.log),
) )
} }
func createSearchSvcV2(sSearch *searchsvc.Service, keyStorage *util.KeyStorage) *searchsvcV2.Service { func createSearchSvcV2(sSearch *searchsvc.Service, keyStorage *util.KeyStorage) *searchsvcV2.Service {
return searchsvcV2.NewService( return searchsvcV2.NewService(sSearch, keyStorage)
searchsvcV2.WithInternalService(sSearch),
searchsvcV2.WithKeyStorage(keyStorage),
)
} }
func createGetService(c *cfg, keyStorage *util.KeyStorage, traverseGen *util.TraverserGenerator, func createGetService(c *cfg, keyStorage *util.KeyStorage, traverseGen *util.TraverserGenerator,
@ -382,24 +374,22 @@ func createGetServiceV2(sGet *getsvc.Service, keyStorage *util.KeyStorage) *gets
func createDeleteService(c *cfg, keyStorage *util.KeyStorage, sGet *getsvc.Service, func createDeleteService(c *cfg, keyStorage *util.KeyStorage, sGet *getsvc.Service,
sSearch *searchsvc.Service, sPut *putsvc.Service) *deletesvc.Service { sSearch *searchsvc.Service, sPut *putsvc.Service) *deletesvc.Service {
return deletesvc.New( return deletesvc.New(
deletesvc.WithLogger(c.log), sGet,
deletesvc.WithHeadService(sGet), sSearch,
deletesvc.WithSearchService(sSearch), sPut,
deletesvc.WithPutService(sPut), &delNetInfo{
deletesvc.WithNetworkInfo(&delNetInfo{
State: c.cfgNetmap.state, State: c.cfgNetmap.state,
tsLifetime: c.cfgObject.tombstoneLifetime, tsLifetime: c.cfgObject.tombstoneLifetime,
cfg: c, cfg: c,
}), },
deletesvc.WithKeyStorage(keyStorage), keyStorage,
deletesvc.WithLogger(c.log),
) )
} }
func createDeleteServiceV2(sDelete *deletesvc.Service) *deletesvcV2.Service { func createDeleteServiceV2(sDelete *deletesvc.Service) *deletesvcV2.Service {
return deletesvcV2.NewService( return deletesvcV2.NewService(sDelete)
deletesvcV2.WithInternalService(sDelete),
)
} }
func createSplitService(c *cfg, sPutV2 *putsvcV2.Service, sGetV2 *getsvcV2.Service, func createSplitService(c *cfg, sPutV2 *putsvcV2.Service, sGetV2 *getsvcV2.Service,
@ -421,21 +411,16 @@ func createACLServiceV2(c *cfg, splitSvc *objectService.TransportSplitter) v2.Se
irFetcher := createInnerRingFetcher(c) irFetcher := createInnerRingFetcher(c)
return v2.New( return v2.New(
v2.WithLogger(c.log), splitSvc,
v2.WithIRFetcher(newCachedIRFetcher(irFetcher)), c.netMapSource,
v2.WithNetmapSource(c.netMapSource), newCachedIRFetcher(irFetcher),
v2.WithContainerSource( acl.NewChecker(
c.cfgNetmap.state,
c.cfgObject.eaclSource,
eaclSDK.NewValidator(),
ls),
c.cfgObject.cnrSource, c.cfgObject.cnrSource,
), v2.WithLogger(c.log),
v2.WithNextService(splitSvc),
v2.WithEACLChecker(
acl.NewChecker(new(acl.CheckerPrm).
SetNetmapState(c.cfgNetmap.state).
SetEACLSource(c.cfgObject.eaclSource).
SetValidator(eaclSDK.NewValidator()).
SetLocalStorage(ls),
),
),
) )
} }

View file

@ -1,10 +1,12 @@
package acl package acl
import ( import (
"context"
"crypto/ecdsa" "crypto/ecdsa"
"crypto/elliptic" "crypto/elliptic"
"errors" "errors"
"fmt" "fmt"
"io"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap"
@ -17,39 +19,12 @@ import (
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
frostfsecdsa "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/crypto/ecdsa" frostfsecdsa "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/crypto/ecdsa"
eaclSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl" eaclSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl"
objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user"
"github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/crypto/keys"
) )
// CheckerPrm groups parameters for Checker
// constructor.
type CheckerPrm struct {
eaclSrc container.EACLSource
validator *eaclSDK.Validator
localStorage *engine.StorageEngine
state netmap.State
}
func (c *CheckerPrm) SetEACLSource(v container.EACLSource) *CheckerPrm {
c.eaclSrc = v
return c
}
func (c *CheckerPrm) SetValidator(v *eaclSDK.Validator) *CheckerPrm {
c.validator = v
return c
}
func (c *CheckerPrm) SetLocalStorage(v *engine.StorageEngine) *CheckerPrm {
c.localStorage = v
return c
}
func (c *CheckerPrm) SetNetmapState(v netmap.State) *CheckerPrm {
c.state = v
return c
}
// Checker implements v2.ACLChecker interfaces and provides // Checker implements v2.ACLChecker interfaces and provides
// ACL/eACL validation functionality. // ACL/eACL validation functionality.
type Checker struct { type Checker struct {
@ -59,6 +34,18 @@ type Checker struct {
state netmap.State state netmap.State
} }
type localStorage struct {
ale64bit marked this conversation as resolved Outdated

why the struct even? just alias it?

why the `struct` even? just alias it?

From here: #495/files

From here: https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/495/files#diff-65b4b70b0453f2a0abd08c354212feeb6a6680d4
ls *engine.StorageEngine
}
func (s *localStorage) Head(ctx context.Context, addr oid.Address) (*objectSDK.Object, error) {
if s.ls == nil {
return nil, io.ErrUnexpectedEOF
ale64bit marked this conversation as resolved Outdated

this might hide a more serious error, no? (i.e. a missing/nil argument somewhere)

this might hide a more serious error, no? (i.e. a missing/nil argument somewhere)

again from here: #495/files#diff-65b4b70b0453f2a0abd08c354212feeb6a6680d4

again from here: #495/files#diff-65b4b70b0453f2a0abd08c354212feeb6a6680d4
}
return engine.Head(ctx, s.ls, addr)
}
// Various EACL check errors. // Various EACL check errors.
var ( var (
errEACLDeniedByRule = errors.New("denied by rule") errEACLDeniedByRule = errors.New("denied by rule")
@ -71,23 +58,17 @@ var (
// NewChecker creates Checker. // NewChecker creates Checker.
// Panics if at least one of the parameter is nil. // Panics if at least one of the parameter is nil.
func NewChecker(prm *CheckerPrm) *Checker { func NewChecker(
panicOnNil := func(fieldName string, field any) { state netmap.State,
if field == nil { eaclSrc container.EACLSource,
panic(fmt.Sprintf("incorrect field %s (%T): %v", fieldName, field, field)) validator *eaclSDK.Validator,
} localStorage *engine.StorageEngine,
} ) *Checker {
panicOnNil("EACLSource", prm.eaclSrc)
panicOnNil("EACLValidator", prm.validator)
panicOnNil("LocalStorageEngine", prm.localStorage)
panicOnNil("NetmapState", prm.state)
return &Checker{ return &Checker{
eaclSrc: prm.eaclSrc, eaclSrc: eaclSrc,
validator: prm.validator, validator: validator,
localStorage: prm.localStorage, localStorage: localStorage,
state: prm.state, state: state,
} }
} }
@ -193,26 +174,14 @@ func getRole(reqInfo v2.RequestInfo) eaclSDK.Role {
} }
func (c *Checker) getHeaderSource(cnr cid.ID, msg any, reqInfo v2.RequestInfo) (eaclSDK.TypedHeaderSource, error) { func (c *Checker) getHeaderSource(cnr cid.ID, msg any, reqInfo v2.RequestInfo) (eaclSDK.TypedHeaderSource, error) {
hdrSrcOpts := make([]eaclV2.Option, 0, 3) var xHeaderSource eaclV2.XHeaderSource
hdrSrcOpts = append(hdrSrcOpts,
eaclV2.WithLocalObjectStorage(c.localStorage),
eaclV2.WithCID(cnr),
eaclV2.WithOID(reqInfo.ObjectID()),
)
if req, ok := msg.(eaclV2.Request); ok { if req, ok := msg.(eaclV2.Request); ok {
hdrSrcOpts = append(hdrSrcOpts, eaclV2.WithServiceRequest(req)) xHeaderSource = eaclV2.NewRequestXHeaderSource(req)
} else { } else {
hdrSrcOpts = append(hdrSrcOpts, xHeaderSource = eaclV2.NewResponseXHeaderSource(msg.(eaclV2.Response), reqInfo.Request().(eaclV2.Request))
eaclV2.WithServiceResponse(
msg.(eaclV2.Response),
reqInfo.Request().(eaclV2.Request),
),
)
} }
hdrSrc, err := eaclV2.NewMessageHeaderSource(hdrSrcOpts...) hdrSrc, err := eaclV2.NewMessageHeaderSource(&localStorage{ls: c.localStorage}, xHeaderSource, cnr, eaclV2.WithOID(reqInfo.ObjectID()))
if err != nil { if err != nil {
return nil, fmt.Errorf("can't parse headers: %w", err) return nil, fmt.Errorf("can't parse headers: %w", err)
} }

View file

@ -27,12 +27,11 @@ func (e emptyNetmapState) CurrentEpoch() uint64 {
} }
func TestStickyCheck(t *testing.T) { func TestStickyCheck(t *testing.T) {
checker := NewChecker(new(CheckerPrm). checker := NewChecker(
SetLocalStorage(&engine.StorageEngine{}). emptyNetmapState{},
SetValidator(eaclSDK.NewValidator()). emptyEACLSource{},
SetEACLSource(emptyEACLSource{}). eaclSDK.NewValidator(),
SetNetmapState(emptyNetmapState{}), &engine.StorageEngine{})
)
t.Run("system role", func(t *testing.T) { t.Run("system role", func(t *testing.T) {
var info v2.RequestInfo var info v2.RequestInfo

View file

@ -103,9 +103,9 @@ func TestHeadRequest(t *testing.T) {
newSource := func(t *testing.T) eaclSDK.TypedHeaderSource { newSource := func(t *testing.T) eaclSDK.TypedHeaderSource {
hdrSrc, err := NewMessageHeaderSource( hdrSrc, err := NewMessageHeaderSource(
WithObjectStorage(lStorage), lStorage,
WithServiceRequest(req), NewRequestXHeaderSource(req),
WithCID(addr.Container()), addr.Container(),
WithOID(&id)) WithOID(&id))
require.NoError(t, err) require.NoError(t, err)
return hdrSrc return hdrSrc

View file

@ -21,7 +21,7 @@ type Option func(*cfg)
type cfg struct { type cfg struct {
storage ObjectStorage storage ObjectStorage
msg xHeaderSource msg XHeaderSource
cnr cid.ID cnr cid.ID
obj *oid.ID obj *oid.ID
@ -46,14 +46,12 @@ type headerSource struct {
incompleteObjectHeaders bool incompleteObjectHeaders bool
} }
func defaultCfg() *cfg { func NewMessageHeaderSource(os ObjectStorage, xhs XHeaderSource, cnrID cid.ID, opts ...Option) (eaclSDK.TypedHeaderSource, error) {
return &cfg{ cfg := &cfg{
storage: new(localStorage), storage: os,
cnr: cnrID,
msg: xhs,
} }
}
func NewMessageHeaderSource(opts ...Option) (eaclSDK.TypedHeaderSource, error) {
cfg := defaultCfg()
for i := range opts { for i := range opts {
opts[i](cfg) opts[i](cfg)
@ -70,7 +68,7 @@ func NewMessageHeaderSource(opts ...Option) (eaclSDK.TypedHeaderSource, error) {
return nil, err return nil, err
} }
res.requestHeaders = requestHeaders(cfg.msg) res.requestHeaders = cfg.msg.GetXHeaders()
return res, nil return res, nil
} }
@ -96,10 +94,6 @@ func (x xHeader) Value() string {
return (*session.XHeader)(&x).GetValue() return (*session.XHeader)(&x).GetValue()
} }
func requestHeaders(msg xHeaderSource) []eaclSDK.Header {
return msg.GetXHeaders()
}
var errMissingOID = errors.New("object ID is missing") var errMissingOID = errors.New("object ID is missing")
func (h *cfg) readObjectHeaders(dst *headerSource) error { func (h *cfg) readObjectHeaders(dst *headerSource) error {

View file

@ -1,22 +0,0 @@
package v2
import (
"context"
"io"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/engine"
objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
)
type localStorage struct {
ls *engine.StorageEngine
}
func (s *localStorage) Head(ctx context.Context, addr oid.Address) (*objectSDK.Object, error) {
if s.ls == nil {
return nil, io.ErrUnexpectedEOF
}
return engine.Head(ctx, s.ls, addr)
}

View file

@ -1,48 +1,9 @@
package v2 package v2
import ( import (
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/engine"
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
) )
func WithObjectStorage(v ObjectStorage) Option {
return func(c *cfg) {
c.storage = v
}
}
func WithLocalObjectStorage(v *engine.StorageEngine) Option {
return func(c *cfg) {
c.storage = &localStorage{
ls: v,
}
}
}
func WithServiceRequest(v Request) Option {
return func(c *cfg) {
c.msg = requestXHeaderSource{
req: v,
}
}
}
func WithServiceResponse(resp Response, req Request) Option {
return func(c *cfg) {
c.msg = responseXHeaderSource{
resp: resp,
req: req,
}
}
}
func WithCID(v cid.ID) Option {
return func(c *cfg) {
c.cnr = v
}
}
func WithOID(v *oid.ID) Option { func WithOID(v *oid.ID) Option {
return func(c *cfg) { return func(c *cfg) {
c.obj = v c.obj = v

View file

@ -5,7 +5,7 @@ import (
eaclSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl" eaclSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl"
) )
type xHeaderSource interface { type XHeaderSource interface {
GetXHeaders() []eaclSDK.Header GetXHeaders() []eaclSDK.Header
} }
@ -13,12 +13,20 @@ type requestXHeaderSource struct {
req Request req Request
} }
func NewRequestXHeaderSource(req Request) XHeaderSource {
return requestXHeaderSource{req: req}
}
type responseXHeaderSource struct { type responseXHeaderSource struct {
resp Response resp Response
req Request req Request
} }
func NewResponseXHeaderSource(resp Response, req Request) XHeaderSource {
return responseXHeaderSource{resp: resp, req: req}
}
func (s requestXHeaderSource) GetXHeaders() []eaclSDK.Header { func (s requestXHeaderSource) GetXHeaders() []eaclSDK.Header {
ln := 0 ln := 0

View file

@ -1,9 +1,6 @@
package v2 package v2
import ( import (
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap"
objectSvc "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger"
) )
@ -13,39 +10,3 @@ func WithLogger(v *logger.Logger) Option {
c.log = v c.log = v
} }
} }
// WithNetmapSource return option to set
// netmap source.
func WithNetmapSource(v netmap.Source) Option {
return func(c *cfg) {
c.nm = v
}
}
// WithContainerSource returns option to set container source.
func WithContainerSource(v container.Source) Option {
return func(c *cfg) {
c.containers = v
}
}
// WithNextService returns option to set next object service.
func WithNextService(v objectSvc.ServiceServer) Option {
return func(c *cfg) {
c.next = v
}
}
// WithEACLChecker returns option to set eACL checker.
func WithEACLChecker(v ACLChecker) Option {
return func(c *cfg) {
c.checker = v
}
}
// WithIRFetcher returns option to set inner ring fetcher.
func WithIRFetcher(v InnerRingFetcher) Option {
return func(c *cfg) {
c.irFetcher = v
}
}

View file

@ -73,32 +73,26 @@ type cfg struct {
next object.ServiceServer next object.ServiceServer
} }
func defaultCfg() *cfg {
return &cfg{
log: &logger.Logger{Logger: zap.L()},
}
}
// New is a constructor for object ACL checking service. // New is a constructor for object ACL checking service.
func New(opts ...Option) Service { func New(next object.ServiceServer,
cfg := defaultCfg() nm netmap.Source,
irf InnerRingFetcher,
acl ACLChecker,
cs container.Source,
opts ...Option) Service {
cfg := &cfg{

I think that New should have log log.Logger parameter that can be used for cfg instance creation. This will support dependency inversion.
From the other side - this barely can be helpful because we do not use mocks

I think that `New` should have `log log.Logger` parameter that can be used for `cfg` instance creation. This will support dependency inversion. From the other side - this barely can be helpful because we do not use mocks

logger can be passed with options, because logger is not mandatory and default logger will be used if it is not proovided

`logger` can be passed with options, because logger is not mandatory and default logger will be used if it is not proovided

Can we remove panicOnNil below?

Can we remove `panicOnNil` below?

done

done
log: &logger.Logger{Logger: zap.L()},
next: next,
nm: nm,
irFetcher: irf,
checker: acl,
containers: cs,
}
for i := range opts { for i := range opts {
opts[i](cfg) opts[i](cfg)
} }
panicOnNil := func(v any, name string) {
if v == nil {
panic(fmt.Sprintf("ACL service: %s is nil", name))
}
}
panicOnNil(cfg.next, "next Service")
panicOnNil(cfg.nm, "netmap client")
panicOnNil(cfg.irFetcher, "inner Ring fetcher")
panicOnNil(cfg.checker, "acl checker")
panicOnNil(cfg.containers, "container source")
return Service{ return Service{
cfg: cfg, cfg: cfg,
c: senderClassifier{ c: senderClassifier{

View file

@ -62,16 +62,22 @@ type cfg struct {
keyStorage *util.KeyStorage keyStorage *util.KeyStorage
} }
func defaultCfg() *cfg {
return &cfg{
log: &logger.Logger{Logger: zap.L()},
}
}
// New creates, initializes and returns utility serving // New creates, initializes and returns utility serving
// Object.Get service requests. // Object.Get service requests.
func New(opts ...Option) *Service { func New(gs *getsvc.Service,
c := defaultCfg() ss *searchsvc.Service,
ps *putsvc.Service,
ni NetworkInfo,
ks *util.KeyStorage,
opts ...Option) *Service {
c := &cfg{
log: &logger.Logger{Logger: zap.L()},
header: &headSvcWrapper{s: gs},
ale64bit marked this conversation as resolved Outdated

do we plan to get rid of these casts that scare adults and children alike in another task/PR?

do we plan to get rid of these casts that scare adults and children alike in another task/PR?

fixed

fixed
searcher: &searchSvcWrapper{s: ss},
placer: &putSvcWrapper{s: ps},
netInfo: ni,
keyStorage: ks,
}
for i := range opts { for i := range opts {
opts[i](c) opts[i](c)
@ -88,39 +94,3 @@ func WithLogger(l *logger.Logger) Option {
c.log = &logger.Logger{Logger: l.With(zap.String("component", "objectSDK.Delete service"))} c.log = &logger.Logger{Logger: l.With(zap.String("component", "objectSDK.Delete service"))}
} }
} }
// WithHeadService returns option to set Head service
// to work with object headers.
func WithHeadService(h *getsvc.Service) Option {
return func(c *cfg) {
c.header = (*headSvcWrapper)(h)
}
}
// WithSearchService returns option to set search service.
func WithSearchService(s *searchsvc.Service) Option {
return func(c *cfg) {
c.searcher = (*searchSvcWrapper)(s)
}
}
// WithPutService returns option to specify put service.
func WithPutService(p *putsvc.Service) Option {
return func(c *cfg) {
c.placer = (*putSvcWrapper)(p)
}
}
// WithNetworkInfo returns option to set network information source.
func WithNetworkInfo(netInfo NetworkInfo) Option {
return func(c *cfg) {
c.netInfo = netInfo
}
}
// WithKeyStorage returns option to set local private key storage.
func WithKeyStorage(ks *util.KeyStorage) Option {
return func(c *cfg) {
c.keyStorage = ks
}
}

View file

@ -11,11 +11,17 @@ import (
oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
) )
type headSvcWrapper getsvc.Service type headSvcWrapper struct {
s *getsvc.Service
}
type searchSvcWrapper searchsvc.Service type searchSvcWrapper struct {
s *searchsvc.Service
}
type putSvcWrapper putsvc.Service type putSvcWrapper struct {
s *putsvc.Service
}
type simpleIDWriter struct { type simpleIDWriter struct {
ids []oid.ID ids []oid.ID
@ -30,7 +36,7 @@ func (w *headSvcWrapper) headAddress(ctx context.Context, exec *execCtx, addr oi
p.WithRawFlag(true) p.WithRawFlag(true)
p.WithAddress(addr) p.WithAddress(addr)
err := (*getsvc.Service)(w).Head(ctx, p) err := w.s.Head(ctx, p)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -94,7 +100,7 @@ func (w *searchSvcWrapper) splitMembers(ctx context.Context, exec *execCtx) ([]o
p.WithContainerID(exec.containerID()) p.WithContainerID(exec.containerID())
p.WithSearchFilters(fs) p.WithSearchFilters(fs)
err := (*searchsvc.Service)(w).Search(ctx, p) err := w.s.Search(ctx, p)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -109,7 +115,7 @@ func (s *simpleIDWriter) WriteIDs(ids []oid.ID) error {
} }
func (w *putSvcWrapper) put(ctx context.Context, exec *execCtx) (*oid.ID, error) { func (w *putSvcWrapper) put(ctx context.Context, exec *execCtx) (*oid.ID, error) {
streamer, err := (*putsvc.Service)(w).Put() streamer, err := w.s.Put()
if err != nil { if err != nil {
return nil, err return nil, err
} }

View file

@ -9,26 +9,13 @@ import (
// Service implements Delete operation of Object service v2. // Service implements Delete operation of Object service v2.
type Service struct { type Service struct {
*cfg
}
// Option represents Service constructor option.
type Option func(*cfg)
type cfg struct {
svc *deletesvc.Service svc *deletesvc.Service
} }
// NewService constructs Service instance from provided options. // NewService constructs Service instance from provided options.
func NewService(opts ...Option) *Service { func NewService(svc *deletesvc.Service) *Service {
c := new(cfg)
for i := range opts {
opts[i](c)
}
return &Service{ return &Service{
cfg: c, svc: svc,
} }
} }
@ -51,9 +38,3 @@ func (s *Service) Delete(ctx context.Context, req *objectV2.DeleteRequest) (*obj
return resp, nil return resp, nil
} }
func WithInternalService(v *deletesvc.Service) Option {
return func(c *cfg) {
c.svc = v
}
}

View file

@ -46,8 +46,6 @@ type cfg struct {
fmtValidator *object.FormatValidator fmtValidator *object.FormatValidator
fmtValidatorOpts []object.FormatValidatorOption
networkState netmap.State networkState netmap.State
clientConstructor ClientConstructor clientConstructor ClientConstructor
@ -55,22 +53,34 @@ type cfg struct {
log *logger.Logger log *logger.Logger
} }
func defaultCfg() *cfg { func NewService(ks *objutil.KeyStorage,
return &cfg{ cc ClientConstructor,
ms MaxSizeSource,
os ObjectStorage,
cs container.Source,
ns netmap.Source,
nk netmap.AnnouncedKeys,
nst netmap.State,
opts ...Option) *Service {
c := &cfg{

Here are inevitably many mandatory parameters. Don't you consider to unite some parameters in one struct?

Just for example

type cfgSource struct {
    cs container.Source
    ns netmap.Source
    ms MaxSizeSource
}
Here are inevitably many mandatory parameters. Don't you consider to unite some parameters in one struct? Just for example ```golang type cfgSource struct { cs container.Source ns netmap.Source ms MaxSizeSource } ```

Let's assume that you decided to add a new dependency. If you use a structure with fields, as you suggested, then you can easily forget to initialize one of the fields, and discover it only during execution.

type NewParams struct {
	Dep1Value   Dep1Type
    Dep2Value   Dep2Type
    NewDepValue NewDepType
}


func New(p *NewParams) *Service {
	s := &Service {
    	dep1: p.Dep1Value,
        dep2: p.Dep2Value,
        dep3: p.NewDepValue
    }
}

func createService() *Service {
	return New(&NewParams{
    			Dep1Value: getDep1(),
                Dep2Value: getDep2(),
                // oops, NewDepValue missed
                })
}

In the case of passing by argument, the source code simply will not be assembled until you pass a new dependency.

Let's assume that you decided to add a new dependency. If you use a structure with fields, as you suggested, then you can easily forget to initialize one of the fields, and discover it only during execution. ``` type NewParams struct { Dep1Value Dep1Type Dep2Value Dep2Type NewDepValue NewDepType } func New(p *NewParams) *Service { s := &Service { dep1: p.Dep1Value, dep2: p.Dep2Value, dep3: p.NewDepValue } } func createService() *Service { return New(&NewParams{ Dep1Value: getDep1(), Dep2Value: getDep2(), // oops, NewDepValue missed }) } ``` In the case of passing by argument, the source code simply will not be assembled until you pass a new dependency.

Another possibility is having a linter which can check whether all struct fields are filled? Like this:

type Prm struct {
	A // param:obligatory
    B // param:obligatory
    C
}
Another possibility is having a linter which can check whether all struct fields are filled? Like this: ``` type Prm struct { A // param:obligatory B // param:obligatory C } ```

This linter must be smart enough to find all if/else branches.

This linter must be smart enough to find all `if/else` branches.

Well, we could enforce providing all values immediately and move all if/else branches above.

Well, we could _enforce_ providing all values immediately and move all if/else branches above.

But obviously needs discussion first.

But obviously needs discussion first.
remotePool: util.NewPseudoWorkerPool(), remotePool: util.NewPseudoWorkerPool(),
localPool: util.NewPseudoWorkerPool(), localPool: util.NewPseudoWorkerPool(),
log: &logger.Logger{Logger: zap.L()}, log: &logger.Logger{Logger: zap.L()},
keyStorage: ks,
clientConstructor: cc,
maxSizeSrc: ms,
localStore: os,
cnrSrc: cs,
netMapSrc: ns,
netmapKeys: nk,
networkState: nst,
} }
}
func NewService(opts ...Option) *Service {
c := defaultCfg()
for i := range opts { for i := range opts {
opts[i](c) opts[i](c)
} }
c.fmtValidator = object.NewFormatValidator(c.fmtValidatorOpts...) c.fmtValidator = object.NewFormatValidator(object.WithLockSource(os), object.WithNetState(nst))
return &Service{ return &Service{
cfg: c, cfg: c,
@ -83,62 +93,12 @@ func (p *Service) Put() (*Streamer, error) {
}, nil }, nil
} }
func WithKeyStorage(v *objutil.KeyStorage) Option {
return func(c *cfg) {
c.keyStorage = v
}
}
func WithMaxSizeSource(v MaxSizeSource) Option {
return func(c *cfg) {
c.maxSizeSrc = v
}
}
func WithObjectStorage(v ObjectStorage) Option {
return func(c *cfg) {
c.localStore = v
c.fmtValidatorOpts = append(c.fmtValidatorOpts, object.WithLockSource(v))
}
}
func WithContainerSource(v container.Source) Option {
return func(c *cfg) {
c.cnrSrc = v
}
}
func WithNetworkMapSource(v netmap.Source) Option {
return func(c *cfg) {
c.netMapSrc = v
}
}
func WithWorkerPools(remote, local util.WorkerPool) Option { func WithWorkerPools(remote, local util.WorkerPool) Option {
return func(c *cfg) { return func(c *cfg) {
c.remotePool, c.localPool = remote, local c.remotePool, c.localPool = remote, local
} }
} }
func WithNetmapKeys(v netmap.AnnouncedKeys) Option {
return func(c *cfg) {
c.netmapKeys = v
}
}
func WithNetworkState(v netmap.State) Option {
return func(c *cfg) {
c.networkState = v
c.fmtValidatorOpts = append(c.fmtValidatorOpts, object.WithNetState(v))
}
}
func WithClientConstructor(v ClientConstructor) Option {
return func(c *cfg) {
c.clientConstructor = v
}
}
func WithLogger(l *logger.Logger) Option { func WithLogger(l *logger.Logger) Option {
return func(c *cfg) { return func(c *cfg) {
c.log = l c.log = l

View file

@ -12,27 +12,15 @@ import (
// Service implements Put operation of Object service v2. // Service implements Put operation of Object service v2.
type Service struct { type Service struct {
*cfg
}
// Option represents Service constructor option.
type Option func(*cfg)
type cfg struct {
svc *putsvc.Service svc *putsvc.Service
keyStorage *util.KeyStorage keyStorage *util.KeyStorage
} }
// NewService constructs Service instance from provided options. // NewService constructs Service instance from provided options.
func NewService(opts ...Option) *Service { func NewService(svc *putsvc.Service, ks *util.KeyStorage) *Service {
c := new(cfg)
for i := range opts {
opts[i](c)
}
return &Service{ return &Service{
cfg: c, svc: svc,
keyStorage: ks,
} }
} }
@ -52,15 +40,3 @@ func (s *Service) Put() (object.PutObjectStream, error) {
func (s *Service) PutSingle(ctx context.Context, req *objectAPI.PutSingleRequest) (*objectAPI.PutSingleResponse, error) { func (s *Service) PutSingle(ctx context.Context, req *objectAPI.PutSingleRequest) (*objectAPI.PutSingleResponse, error) {
return s.svc.PutSingle(ctx, req) return s.svc.PutSingle(ctx, req)
} }
func WithInternalService(v *putsvc.Service) Option {
return func(c *cfg) {
c.svc = v
}
}
func WithKeyStorage(ks *util.KeyStorage) Option {
return func(c *cfg) {
c.keyStorage = ks
}
}

View file

@ -55,17 +55,28 @@ type cfg struct {
keyStore *util.KeyStorage keyStore *util.KeyStorage
} }
func defaultCfg() *cfg {
return &cfg{
log: &logger.Logger{Logger: zap.L()},
clientConstructor: new(clientConstructorWrapper),
}
}
// New creates, initializes and returns utility serving // New creates, initializes and returns utility serving
// Object.Get service requests. // Object.Get service requests.
func New(opts ...Option) *Service { func New(e *engine.StorageEngine,
c := defaultCfg() cc ClientConstructor,
tg *util.TraverserGenerator,
ns netmap.Source,
ks *util.KeyStorage,
opts ...Option) *Service {
c := &cfg{
log: &logger.Logger{Logger: zap.L()},
clientConstructor: &clientConstructorWrapper{
constructor: cc,
},
localStorage: &storageEngineWrapper{
storage: e,
},
traverserGenerator: (*traverseGeneratorWrapper)(tg),
currentEpochReceiver: &nmSrcWrapper{
nmSrc: ns,
},
keyStore: ks,
}
for i := range opts { for i := range opts {
opts[i](c) opts[i](c)
@ -82,46 +93,3 @@ func WithLogger(l *logger.Logger) Option {
c.log = &logger.Logger{Logger: l.With(zap.String("component", "Object.Search service"))} c.log = &logger.Logger{Logger: l.With(zap.String("component", "Object.Search service"))}
} }
} }
// WithLocalStorageEngine returns option to set local storage
// instance.
func WithLocalStorageEngine(e *engine.StorageEngine) Option {
return func(c *cfg) {
c.localStorage = &storageEngineWrapper{
storage: e,
}
}
}
// WithClientConstructor returns option to set constructor of remote node clients.
func WithClientConstructor(v ClientConstructor) Option {
return func(c *cfg) {
c.clientConstructor.(*clientConstructorWrapper).constructor = v
}
}
// WithTraverserGenerator returns option to set generator of
// placement traverser to get the objects from containers.
func WithTraverserGenerator(t *util.TraverserGenerator) Option {
return func(c *cfg) {
c.traverserGenerator = (*traverseGeneratorWrapper)(t)
}
}
// WithNetMapSource returns option to set network
// map storage to receive current network state.
func WithNetMapSource(nmSrc netmap.Source) Option {
return func(c *cfg) {
c.currentEpochReceiver = &nmSrcWrapper{
nmSrc: nmSrc,
}
}
}
// WithKeyStorage returns option to set private
// key storage for session tokens and node key.
func WithKeyStorage(store *util.KeyStorage) Option {
return func(c *cfg) {
c.keyStore = store
}
}

View file

@ -9,28 +9,15 @@ import (
// Service implements Search operation of Object service v2. // Service implements Search operation of Object service v2.
type Service struct { type Service struct {
*cfg
}
// Option represents Service constructor option.
type Option func(*cfg)
type cfg struct {
svc *searchsvc.Service svc *searchsvc.Service
keyStorage *objutil.KeyStorage keyStorage *objutil.KeyStorage
} }
// NewService constructs Service instance from provided options. // NewService constructs Service instance from provided options.
func NewService(opts ...Option) *Service { func NewService(s *searchsvc.Service, ks *objutil.KeyStorage) *Service {
c := new(cfg)
for i := range opts {
opts[i](c)
}
return &Service{ return &Service{
cfg: c, svc: s,
keyStorage: ks,
} }
} }
@ -43,18 +30,3 @@ func (s *Service) Search(req *objectV2.SearchRequest, stream objectSvc.SearchStr
return s.svc.Search(stream.Context(), *p) return s.svc.Search(stream.Context(), *p)
} }
// WithInternalService returns option to set entity
// that handles request payload.
func WithInternalService(v *searchsvc.Service) Option {
return func(c *cfg) {
c.svc = v
}
}
// WithKeyStorage returns option to set local private key storage.
func WithKeyStorage(ks *objutil.KeyStorage) Option {
return func(c *cfg) {
c.keyStorage = ks
}
}