From 936e6d230b74a22a060ea89f3810ff7e01320a15 Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Thu, 3 Aug 2023 12:14:00 +0300 Subject: [PATCH] [#121] pool: Add wait params validation for containerPut method * Add WaitParams.CheckValidity() check because SetWaitParams is deprecated, but parameters were checked within this setter with checkForPositive() Signed-off-by: Airat Arifullin a.arifullin@yadro.com --- client/container_delete.go | 8 ++++---- pool/pool.go | 38 +++++++++++++++++++++++--------------- pool/pool_test.go | 12 ++++++------ 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/client/container_delete.go b/client/container_delete.go index 70d5f40..181c15b 100644 --- a/client/container_delete.go +++ b/client/container_delete.go @@ -30,8 +30,8 @@ type PrmContainerDelete struct { // Required parameter. // // Deprecated: Use PrmContainerDelete.Container instead. -func (x *PrmContainerDelete) SetContainer(id cid.ID) { - x.ContainerID = &id +func (prm *PrmContainerDelete) SetContainer(id cid.ID) { + prm.ContainerID = &id } func (prm *PrmContainerDelete) buildRequest(c *Client) (*v2container.DeleteRequest, error) { @@ -88,8 +88,8 @@ func (prm *PrmContainerDelete) buildRequest(c *Client) (*v2container.DeleteReque // Must be signed. // // Deprecated: Use PrmContainerDelete.Session instead. -func (x *PrmContainerDelete) WithinSession(tok session.Container) { - x.Session = &tok +func (prm *PrmContainerDelete) WithinSession(tok session.Container) { + prm.Session = &tok } // ResContainerDelete groups resulting values of ContainerDelete operation. diff --git a/pool/pool.go b/pool/pool.go index 31bf84f..075ba7f 100644 --- a/pool/pool.go +++ b/pool/pool.go @@ -420,6 +420,9 @@ func (c *clientWrapper) containerPut(ctx context.Context, prm PrmContainerPut) ( if prm.WaitParams == nil { prm.WaitParams = defaultWaitParams() } + if err = prm.WaitParams.CheckValidity(); err != nil { + return cid.ID{}, fmt.Errorf("invalid wait parameters: %w", err) + } idCnr := res.ID() @@ -1205,45 +1208,50 @@ func (x *NodeParam) Weight() float64 { // WaitParams contains parameters used in polling is a something applied on FrostFS network. type WaitParams struct { - timeout time.Duration - pollInterval time.Duration + Timeout time.Duration + PollInterval time.Duration } // SetTimeout specifies the time to wait for the operation to complete. +// +// Deprecated: Use WaitParams.Timeout instead. func (x *WaitParams) SetTimeout(timeout time.Duration) { - x.timeout = timeout + x.Timeout = timeout } // SetPollInterval specifies the interval, once it will check the completion of the operation. +// +// Deprecated: Use WaitParams.PollInterval instead. func (x *WaitParams) SetPollInterval(tick time.Duration) { - x.pollInterval = tick + x.PollInterval = tick } +// Deprecated: Use defaultWaitParams() instead. func (x *WaitParams) setDefaults() { - x.timeout = 120 * time.Second - x.pollInterval = 5 * time.Second + x.Timeout = 120 * time.Second + x.PollInterval = 5 * time.Second } func defaultWaitParams() *WaitParams { return &WaitParams{ - timeout: 120 * time.Second, - pollInterval: 5 * time.Second, + Timeout: 120 * time.Second, + PollInterval: 5 * time.Second, } } // checkForPositive panics if any of the wait params isn't positive. func (x *WaitParams) checkForPositive() { - if x.timeout <= 0 || x.pollInterval <= 0 { + if x.Timeout <= 0 || x.PollInterval <= 0 { panic("all wait params must be positive") } } // CheckForValid checks if all wait params are non-negative. -func (waitPrm *WaitParams) CheckValidity() error { - if waitPrm.timeout <= 0 { +func (x *WaitParams) CheckValidity() error { + if x.Timeout <= 0 { return errors.New("timeout cannot be negative") } - if waitPrm.pollInterval <= 0 { + if x.PollInterval <= 0 { return errors.New("poll interval cannot be negative") } return nil @@ -2533,9 +2541,9 @@ func waitForContainerRemoved(ctx context.Context, cli client, cnrID *cid.ID, wai // waitFor await that given condition will be met in waitParams time. func waitFor(ctx context.Context, params *WaitParams, condition func(context.Context) bool) error { - wctx, cancel := context.WithTimeout(ctx, params.timeout) + wctx, cancel := context.WithTimeout(ctx, params.Timeout) defer cancel() - ticker := time.NewTimer(params.pollInterval) + ticker := time.NewTimer(params.PollInterval) defer ticker.Stop() wdone := wctx.Done() done := ctx.Done() @@ -2549,7 +2557,7 @@ func waitFor(ctx context.Context, params *WaitParams, condition func(context.Con if condition(ctx) { return nil } - ticker.Reset(params.pollInterval) + ticker.Reset(params.PollInterval) } } } diff --git a/pool/pool_test.go b/pool/pool_test.go index 623d429..08de667 100644 --- a/pool/pool_test.go +++ b/pool/pool_test.go @@ -483,8 +483,8 @@ func TestWaitPresence(t *testing.T) { var idCnr cid.ID err := waitForContainerPresence(ctx, mockCli, idCnr, &WaitParams{ - timeout: 120 * time.Second, - pollInterval: 5 * time.Second, + Timeout: 120 * time.Second, + PollInterval: 5 * time.Second, }) require.Error(t, err) require.Contains(t, err.Error(), "context canceled") @@ -494,8 +494,8 @@ func TestWaitPresence(t *testing.T) { ctx := context.Background() var idCnr cid.ID err := waitForContainerPresence(ctx, mockCli, idCnr, &WaitParams{ - timeout: 500 * time.Millisecond, - pollInterval: 5 * time.Second, + Timeout: 500 * time.Millisecond, + PollInterval: 5 * time.Second, }) require.Error(t, err) require.Contains(t, err.Error(), "context deadline exceeded") @@ -505,8 +505,8 @@ func TestWaitPresence(t *testing.T) { ctx := context.Background() var idCnr cid.ID err := waitForContainerPresence(ctx, mockCli, idCnr, &WaitParams{ - timeout: 10 * time.Second, - pollInterval: 500 * time.Millisecond, + Timeout: 10 * time.Second, + PollInterval: 500 * time.Millisecond, }) require.NoError(t, err) })