TrueCloudLab/frostfs-sdk-go
30
0
Fork 21
Code Issues 30 Pull requests 3 Releases Activity Actions

[#121] client: Make PrmContainerDelete fields public #141

Merged
fyrchik merged 3 commits from aarifullin/frostfs-sdk-go:master into master 2023-08-05 15:58:16 +00:00
Conversation 1 Commits 3 Files changed 3 +80 -52
3 changed files with 80 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
client/container_delete.go
View file

@ -18,29 +18,33 @@ import (
// PrmContainerDelete groups parameters of ContainerDelete operation. // PrmContainerDelete groups parameters of ContainerDelete operation.
type PrmContainerDelete struct { type PrmContainerDelete struct {
prmCommonMeta // FrostFS request X-Headers
XHeaders []string
idSet bool ContainerID *cid.ID
id cid.ID
tokSet bool Session *session.Container
tok session.Container
} }
// SetContainer sets identifier of the FrostFS container to be removed. // SetContainer sets identifier of the FrostFS container to be removed.
// Required parameter. // Required parameter.
func (x *PrmContainerDelete) SetContainer(id cid.ID) { //
x.id = id // Deprecated: Use PrmContainerDelete.Container instead.
x.idSet = true func (prm *PrmContainerDelete) SetContainer(id cid.ID) {
prm.ContainerID = &id
} }
func (x *PrmContainerDelete) buildRequest(c *Client) (*v2container.DeleteRequest, error) { func (prm *PrmContainerDelete) buildRequest(c *Client) (*v2container.DeleteRequest, error) {
if !x.idSet { if prm.ContainerID == nil {
return nil, errorMissingContainer return nil, errorMissingContainer
} }
if len(prm.XHeaders)%2 != 0 {
return nil, errorInvalidXHeaders
}
var cidV2 refs.ContainerID var cidV2 refs.ContainerID
x.id.WriteToV2(&cidV2) prm.ContainerID.WriteToV2(&cidV2)
// Container contract expects signature of container ID value, // Container contract expects signature of container ID value,
// don't get confused with stable marshaled protobuf container.ID structure. // don't get confused with stable marshaled protobuf container.ID structure.
@ -61,11 +65,11 @@ func (x *PrmContainerDelete) buildRequest(c *Client) (*v2container.DeleteRequest
reqBody.SetSignature(&sigv2) reqBody.SetSignature(&sigv2)
var meta v2session.RequestMetaHeader var meta v2session.RequestMetaHeader
writeXHeadersToMeta(x.prmCommonMeta.xHeaders, &meta) writeXHeadersToMeta(prm.XHeaders, &meta)
if x.tokSet { if prm.Session != nil {
var tokv2 v2session.Token var tokv2 v2session.Token
x.tok.WriteToV2(&tokv2) prm.Session.WriteToV2(&tokv2)
meta.SetSessionToken(&tokv2) meta.SetSessionToken(&tokv2)
} }
@ -82,9 +86,10 @@ func (x *PrmContainerDelete) buildRequest(c *Client) (*v2container.DeleteRequest
// This may affect the execution of an operation (e.g. access control). // This may affect the execution of an operation (e.g. access control).
// //
// Must be signed. // Must be signed.
func (x *PrmContainerDelete) WithinSession(tok session.Container) { //
x.tok = tok // Deprecated: Use PrmContainerDelete.Session instead.
x.tokSet = true func (prm *PrmContainerDelete) WithinSession(tok session.Container) {
prm.Session = &tok
} }
// ResContainerDelete groups resulting values of ContainerDelete operation. // ResContainerDelete groups resulting values of ContainerDelete operation.

81
pool/pool.go
View file

@ -420,6 +420,9 @@ func (c *clientWrapper) containerPut(ctx context.Context, prm PrmContainerPut) (
if prm.WaitParams == nil { if prm.WaitParams == nil {
prm.WaitParams = defaultWaitParams() prm.WaitParams = defaultWaitParams()
} }
if err = prm.WaitParams.CheckValidity(); err != nil {
return cid.ID{}, fmt.Errorf("invalid wait parameters: %w", err)
}
idCnr := res.ID() idCnr := res.ID()
@ -486,10 +489,9 @@ func (c *clientWrapper) containerDelete(ctx context.Context, prm PrmContainerDel
return err return err
} }
var cliPrm sdkClient.PrmContainerDelete cliPrm := sdkClient.PrmContainerDelete{
cliPrm.SetContainer(prm.cnrID) ContainerID: &prm.ContainerID,
if prm.stokenSet { Session: prm.Session,
cliPrm.WithinSession(prm.stoken)
} }
start := time.Now() start := time.Now()
@ -503,11 +505,14 @@ func (c *clientWrapper) containerDelete(ctx context.Context, prm PrmContainerDel
return fmt.Errorf("container delete on client: %w", err) return fmt.Errorf("container delete on client: %w", err)
} }
if !prm.waitParamsSet { if prm.WaitParams == nil {
fyrchik marked this conversation as resolved
fyrchik commented 2023-08-03 10:59:43 +00:00
Review
Copy link

Won't private fields be a problem here too?

Won't private fields be a problem here too?
aarifullin commented 2023-08-03 12:42:01 +00:00
Review
Copy link

Sorry, I didn't get the point. prm.WaitParams is no longer private

Sorry, I didn't get the point. `prm.WaitParams` is no longer private
fyrchik commented 2023-08-03 12:45:29 +00:00
Review
Copy link

Yes, but the WaitParams itself has private fields. I think making them public aligns well with the overall goal.

Yes, but the `WaitParams` itself has private fields. I think making them public aligns well with the overall goal.
aarifullin commented 2023-08-03 12:55:08 +00:00
Review
Copy link

Oh. You're right. I'll fix it

Oh. You're right. I'll fix it
prm.waitParams.setDefaults() prm.WaitParams = defaultWaitParams()
}
if err := prm.WaitParams.CheckValidity(); err != nil {
return fmt.Errorf("invalid wait parameters: %w", err)
} }
return waitForContainerRemoved(ctx, c, &prm.cnrID, &prm.waitParams) return waitForContainerRemoved(ctx, c, &prm.ContainerID, prm.WaitParams)
} }
// containerEACL invokes sdkClient.ContainerEACL parse response status to error and return result as is. // containerEACL invokes sdkClient.ContainerEACL parse response status to error and return result as is.
@ -1202,39 +1207,55 @@ func (x *NodeParam) Weight() float64 {
// WaitParams contains parameters used in polling is a something applied on FrostFS network. // WaitParams contains parameters used in polling is a something applied on FrostFS network.
type WaitParams struct { type WaitParams struct {
timeout time.Duration Timeout time.Duration
pollInterval time.Duration PollInterval time.Duration
} }
// SetTimeout specifies the time to wait for the operation to complete. // SetTimeout specifies the time to wait for the operation to complete.
//
// Deprecated: Use WaitParams.Timeout instead.
func (x *WaitParams) SetTimeout(timeout time.Duration) { 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. // 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) { func (x *WaitParams) SetPollInterval(tick time.Duration) {
x.pollInterval = tick x.PollInterval = tick
} }
// Deprecated: Use defaultWaitParams() instead.
func (x *WaitParams) setDefaults() { func (x *WaitParams) setDefaults() {
x.timeout = 120 * time.Second x.Timeout = 120 * time.Second
x.pollInterval = 5 * time.Second x.PollInterval = 5 * time.Second
} }
func defaultWaitParams() *WaitParams { func defaultWaitParams() *WaitParams {
return &WaitParams{ return &WaitParams{
timeout: 120 * time.Second, Timeout: 120 * time.Second,
pollInterval: 5 * time.Second, PollInterval: 5 * time.Second,
} }
} }
// checkForPositive panics if any of the wait params isn't positive. // checkForPositive panics if any of the wait params isn't positive.
func (x *WaitParams) checkForPositive() { 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") panic("all wait params must be positive")
} }
} }
// CheckForValid checks if all wait params are non-negative.
func (x *WaitParams) CheckValidity() error {
if x.Timeout <= 0 {
return errors.New("timeout cannot be negative")
}
if x.PollInterval <= 0 {
return errors.New("poll interval cannot be negative")
}
return nil
}
type prmContext struct { type prmContext struct {
defaultSession bool defaultSession bool
verb session.ObjectVerb verb session.ObjectVerb
@ -1464,33 +1485,35 @@ func (x *PrmContainerList) SetOwnerID(ownerID user.ID) {
// PrmContainerDelete groups parameters of DeleteContainer operation. // PrmContainerDelete groups parameters of DeleteContainer operation.
type PrmContainerDelete struct { type PrmContainerDelete struct {
cnrID cid.ID ContainerID cid.ID
stoken session.Container Session *session.Container
stokenSet bool
waitParams WaitParams WaitParams *WaitParams
waitParamsSet bool
} }
// SetContainerID specifies identifier of the FrostFS container to be removed. // SetContainerID specifies identifier of the FrostFS container to be removed.
//
// Deprecated: Use PrmContainerDelete.ContainerID instead.
func (x *PrmContainerDelete) SetContainerID(cnrID cid.ID) { func (x *PrmContainerDelete) SetContainerID(cnrID cid.ID) {
x.cnrID = cnrID x.ContainerID = cnrID
} }
// SetSessionToken specifies session within which operation should be performed. // SetSessionToken specifies session within which operation should be performed.
//
// Deprecated: Use PrmContainerDelete.Session instead.
func (x *PrmContainerDelete) SetSessionToken(token session.Container) { func (x *PrmContainerDelete) SetSessionToken(token session.Container) {
x.stoken = token x.Session = &token
x.stokenSet = true
} }
// SetWaitParams specifies timeout params to complete operation. // SetWaitParams specifies timeout params to complete operation.
// If not provided the default one will be used. // If not provided the default one will be used.
// Panics if any of the wait params isn't positive. // Panics if any of the wait params isn't positive.
//
// Deprecated: Use PrmContainerDelete.WaitParams instead.
func (x *PrmContainerDelete) SetWaitParams(waitParams WaitParams) { func (x *PrmContainerDelete) SetWaitParams(waitParams WaitParams) {
waitParams.checkForPositive() waitParams.checkForPositive()
x.waitParams = waitParams x.WaitParams = &waitParams
x.waitParamsSet = true
} }
// PrmContainerEACL groups parameters of GetEACL operation. // PrmContainerEACL groups parameters of GetEACL operation.
@ -2515,9 +2538,9 @@ func waitForContainerRemoved(ctx context.Context, cli client, cnrID *cid.ID, wai
// waitFor await that given condition will be met in waitParams time. // waitFor await that given condition will be met in waitParams time.
func waitFor(ctx context.Context, params *WaitParams, condition func(context.Context) bool) error { 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() defer cancel()
ticker := time.NewTimer(params.pollInterval) ticker := time.NewTimer(params.PollInterval)
defer ticker.Stop() defer ticker.Stop()
wdone := wctx.Done() wdone := wctx.Done()
done := ctx.Done() done := ctx.Done()
@ -2531,7 +2554,7 @@ func waitFor(ctx context.Context, params *WaitParams, condition func(context.Con
if condition(ctx) { if condition(ctx) {
return nil return nil
} }
ticker.Reset(params.pollInterval) ticker.Reset(params.PollInterval)
} }
} }
} }

12
pool/pool_test.go
View file

@ -483,8 +483,8 @@ func TestWaitPresence(t *testing.T) {
var idCnr cid.ID var idCnr cid.ID
err := waitForContainerPresence(ctx, mockCli, idCnr, &WaitParams{ err := waitForContainerPresence(ctx, mockCli, idCnr, &WaitParams{
timeout: 120 * time.Second, Timeout: 120 * time.Second,
pollInterval: 5 * time.Second, PollInterval: 5 * time.Second,
}) })
require.Error(t, err) require.Error(t, err)
require.Contains(t, err.Error(), "context canceled") require.Contains(t, err.Error(), "context canceled")
@ -494,8 +494,8 @@ func TestWaitPresence(t *testing.T) {
ctx := context.Background() ctx := context.Background()
var idCnr cid.ID var idCnr cid.ID
err := waitForContainerPresence(ctx, mockCli, idCnr, &WaitParams{ err := waitForContainerPresence(ctx, mockCli, idCnr, &WaitParams{
timeout: 500 * time.Millisecond, Timeout: 500 * time.Millisecond,
pollInterval: 5 * time.Second, PollInterval: 5 * time.Second,
}) })
require.Error(t, err) require.Error(t, err)
require.Contains(t, err.Error(), "context deadline exceeded") require.Contains(t, err.Error(), "context deadline exceeded")
@ -505,8 +505,8 @@ func TestWaitPresence(t *testing.T) {
ctx := context.Background() ctx := context.Background()
var idCnr cid.ID var idCnr cid.ID
err := waitForContainerPresence(ctx, mockCli, idCnr, &WaitParams{ err := waitForContainerPresence(ctx, mockCli, idCnr, &WaitParams{
timeout: 10 * time.Second, Timeout: 10 * time.Second,
pollInterval: 500 * time.Millisecond, PollInterval: 500 * time.Millisecond,
}) })
require.NoError(t, err) require.NoError(t, err)
}) })