Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor handling of setting file expiry policy #21378

Merged
merged 3 commits into from
Aug 16, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions sdk/storage/azdatalake/file/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ import (
"bytes"
"context"
"errors"
"io"
"net/http"
"net/url"
"os"
"reflect"
"strings"
"sync"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
Expand All @@ -25,13 +34,6 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/path"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/shared"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/sas"
"io"
"net/http"
"net/url"
"os"
"strings"
"sync"
"time"
)

// ClientOptions contains the optional parameters when creating a Client.
Expand Down Expand Up @@ -259,9 +261,15 @@ func (f *Client) Rename(ctx context.Context, newName string, options *RenameOpti
}

// SetExpiry operation sets an expiry time on an existing file (blob2).
func (f *Client) SetExpiry(ctx context.Context, expiryType SetExpiryType, o *SetExpiryOptions) (SetExpiryResponse, error) {
expMode, opts := expiryType.Format(o)
resp, err := f.generatedFileClientWithBlob().SetExpiry(ctx, expMode, opts)
func (f *Client) SetExpiry(ctx context.Context, expiryValues SetExpiryValues, o *SetExpiryOptions) (SetExpiryResponse, error) {
if reflect.ValueOf(expiryValues).IsZero() {
expiryValues.ExpiryType = SetExpiryTypeNeverExpire
}
opts := &generated_blob.BlobClientSetExpiryOptions{}
if expiryValues.ExpiryType != SetExpiryTypeNeverExpire {
opts.ExpiresOn = &expiryValues.ExpiresOn
}
resp, err := f.generatedFileClientWithBlob().SetExpiry(ctx, expiryValues.ExpiryType, opts)
err = exported.ConvertToDFSError(err)
return resp, err
}
Expand Down
33 changes: 21 additions & 12 deletions sdk/storage/azdatalake/file/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ import (
"context"
"crypto/md5"
"encoding/binary"
"hash/crc64"
"io"
"math/rand"
"net/http"
"os"
"strconv"
"testing"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/streaming"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"github.com/Azure/azure-sdk-for-go/sdk/internal/recording"
Expand All @@ -21,13 +30,6 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/sas"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"hash/crc64"
"io"
"math/rand"
"net/http"
"os"
"testing"
"time"
)

var proposedLeaseIDs = []*string{to.Ptr("c820a799-76d7-4ee2-6e15-546f19325c2c"), to.Ptr("326cc5e1-746e-4af8-4811-a50e6629a8ca")}
Expand Down Expand Up @@ -472,9 +474,11 @@ func (s *UnrecordedTestSuite) TestCreateFileWithExpiryAbsolute() {
defer testcommon.DeleteFilesystem(context.Background(), _require, fsClient)

expiryTimeAbsolute := time.Now().Add(8 * time.Second)
expiry := file.CreationExpiryTypeAbsolute(expiryTimeAbsolute)
createFileOpts := &file.CreateOptions{
Expiry: expiry,
Expiry: file.CreateExpiryValues{
ExpiresOn: time.Now().Add(8 * time.Second).UTC().Format(http.TimeFormat),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this does put the onus on the developer to get this correct. We could add helper funcs to create a well-formed instance. Maybe we do this later based on customer feedback.

Copy link
Contributor

@tasherif-msft tasherif-msft Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, let's see customer feedback. I will include examples of this in my PR after we merge this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just reverse the initialization order because the mental model is:

  1. Select an expiry type first and then
  2. Set ExpiresOn (if necessary)

ExpiryType: file.CreateExpiryTypeAbsolute,
},
}

_, err = fsClient.Create(context.Background(), nil)
Expand Down Expand Up @@ -505,9 +509,11 @@ func (s *RecordedTestSuite) TestCreateFileWithExpiryRelativeToNow() {
_require.NoError(err)
defer testcommon.DeleteFilesystem(context.Background(), _require, fsClient)

expiry := file.CreationExpiryTypeRelativeToNow(8 * time.Second)
createFileOpts := &file.CreateOptions{
Expiry: expiry,
Expiry: file.CreateExpiryValues{
ExpiresOn: strconv.FormatInt((8 * time.Second).Milliseconds(), 10),
ExpiryType: file.CreateExpiryTypeRelativeToNow,
jhendrixMSFT marked this conversation as resolved.
Show resolved Hide resolved
},
}

_, err = fsClient.Create(context.Background(), nil)
Expand Down Expand Up @@ -540,7 +546,9 @@ func (s *RecordedTestSuite) TestCreateFileWithNeverExpire() {
defer testcommon.DeleteFilesystem(context.Background(), _require, fsClient)

createFileOpts := &file.CreateOptions{
Expiry: file.CreationExpiryTypeNever{},
Expiry: file.CreateExpiryValues{
ExpiryType: file.CreateExpiryTypeNeverExpire,
},
}

_, err = fsClient.Create(context.Background(), nil)
Expand Down Expand Up @@ -2961,6 +2969,7 @@ func (s *RecordedTestSuite) TestFileAppendAndFlushAndDownloadDataWithLeasedFile(
_require.Nil(err)

_, err = rsc.Seek(0, io.SeekStart)
_require.NoError(err)

_, err = srcFClient.AppendData(context.Background(), int64(contentSize), rsc, opts)
_require.Nil(err)
Expand Down
36 changes: 36 additions & 0 deletions sdk/storage/azdatalake/file/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ package file
import (
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/exported"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/generated"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/generated_blob"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/path"
)

Expand Down Expand Up @@ -50,6 +52,40 @@ func TransferValidationTypeComputeCRC64() TransferValidationType {
return exported.TransferValidationTypeComputeCRC64()
}

// SetExpiryType defines the values for modes of file expiration.
type SetExpiryType = generated_blob.ExpiryOptions

const (
// SetExpiryTypeAbsolute sets the expiration date as an absolute value expressed in RFC1123 format.
SetExpiryTypeAbsolute SetExpiryType = generated_blob.ExpiryOptionsAbsolute

// SetExpiryTypeNeverExpire sets the file to never expire or removes the current expiration date.
SetExpiryTypeNeverExpire SetExpiryType = generated_blob.ExpiryOptionsNeverExpire

// SetExpiryTypeRelativeToCreation sets the expiration date relative to the time of file creation.
// The value is expressed as the number of miliseconds to elapse from the time of creation.
SetExpiryTypeRelativeToCreation SetExpiryType = generated_blob.ExpiryOptionsRelativeToCreation

// SetExpiryTypeRelativeToNow sets the expiration date relative to the current time.
// The value is expressed as the number of milliseconds to elapse from the present time.
SetExpiryTypeRelativeToNow SetExpiryType = generated_blob.ExpiryOptionsRelativeToNow
)

// CreateExpiryType defines the values for modes of file expiration specified during creation.
type CreateExpiryType = generated.PathExpiryOptions

const (
// CreateExpiryTypeAbsolute sets the expiration date as an absolute value expressed in RFC1123 format.
CreateExpiryTypeAbsolute CreateExpiryType = generated.PathExpiryOptionsAbsolute

// CreateExpiryTypeNeverExpire sets the file to never expire or removes the current expiration date.
CreateExpiryTypeNeverExpire CreateExpiryType = generated.PathExpiryOptionsNeverExpire

// CreateExpiryTypeRelativeToNow sets the expiration date relative to the current time.
// The value is expressed as the number of milliseconds to elapse from the present time.
CreateExpiryTypeRelativeToNow CreateExpiryType = generated.PathExpiryOptionsRelativeToNow
)

// StatusType defines values for StatusType
type StatusType = azdatalake.StatusType

Expand Down
95 changes: 33 additions & 62 deletions sdk/storage/azdatalake/file/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,14 @@ package file

import (
"errors"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"io"
"reflect"

"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/blob"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/exported"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/generated"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/path"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/shared"
"io"
"net/http"
"strconv"
"time"
)

const (
Expand All @@ -42,7 +40,7 @@ type CreateOptions struct {
// HTTPHeaders contains the HTTP headers for path operations.
HTTPHeaders *HTTPHeaders
// Expiry specifies the type and time of expiry for the file.
Expiry CreationExpiryType
Expiry CreateExpiryValues
// LeaseDuration specifies the duration of the lease, in seconds, or negative one
// (-1) for a lease that never expires. A non-infinite lease can be
// between 15 and 60 seconds.
Expand All @@ -61,6 +59,18 @@ type CreateOptions struct {
ACL *string
}

// CreateExpiryValues describes when a newly created file should expire.
// A zero-value indicates the file has no expiration date.
type CreateExpiryValues struct {
// ExpiresOn contains the time the file should expire.
jhendrixMSFT marked this conversation as resolved.
Show resolved Hide resolved
// The value will either be an absolute UTC time in RFC1123 format or an integer expressing a number of milliseconds.
// NOTE: when ExpiryType is CreateExpiryTypeNeverExpire, this value is ignored.
ExpiresOn string

// ExpiryType indicates how the value of ExpiresOn should be interpreted (absolute, relative to now, etc).
ExpiryType CreateExpiryType
}

func (o *CreateOptions) format() (*generated.LeaseAccessConditions, *generated.ModifiedAccessConditions, *generated.PathHTTPHeaders, *generated.PathClientCreateOptions, *generated.CPKInfo) {
resource := generated.PathResourceTypeFile
createOpts := &generated.PathClientCreateOptions{
Expand All @@ -70,13 +80,11 @@ func (o *CreateOptions) format() (*generated.LeaseAccessConditions, *generated.M
return nil, nil, nil, createOpts, nil
}
leaseAccessConditions, modifiedAccessConditions := exported.FormatPathAccessConditions(o.AccessConditions)
if o.Expiry == nil {
createOpts.ExpiryOptions = nil
createOpts.ExpiresOn = nil
} else {
expOpts, expiresOn := o.Expiry.Format()
createOpts.ExpiryOptions = (*generated.PathExpiryOptions)(&expOpts)
createOpts.ExpiresOn = expiresOn
if !reflect.ValueOf(o.Expiry).IsZero() {
createOpts.ExpiryOptions = &o.Expiry.ExpiryType
if o.Expiry.ExpiryType != CreateExpiryTypeNeverExpire {
createOpts.ExpiresOn = &o.Expiry.ExpiresOn
}
}
createOpts.ACL = o.ACL
createOpts.Group = o.Group
Expand Down Expand Up @@ -491,65 +499,28 @@ func (o *DownloadFileOptions) format() *blob.DownloadFileOptions {
return downloadFileOptions
}

// CreationExpiryType defines values for Create() ExpiryType
type CreationExpiryType interface {
Format() (generated.ExpiryOptions, *string)
notPubliclyImplementable()
}

// CreationExpiryTypeAbsolute defines the absolute time for the blob expiry
type CreationExpiryTypeAbsolute time.Time

// CreationExpiryTypeRelativeToNow defines the duration relative to now for the blob expiry
type CreationExpiryTypeRelativeToNow time.Duration

// CreationExpiryTypeNever defines that the blob will be set to never expire
type CreationExpiryTypeNever struct {
// empty struct since NeverExpire expiry type does not require expiry time
}

func (e CreationExpiryTypeAbsolute) Format() (generated.ExpiryOptions, *string) {
return generated.ExpiryOptionsAbsolute, to.Ptr(time.Time(e).UTC().Format(http.TimeFormat))
// SetExpiryValues describes when a file should expire.
// A zero-value indicates the file has no expiration date.
type SetExpiryValues struct {
// ExpiresOn contains the time the file should expire.
// The value will either be an absolute UTC time in RFC1123 format or an integer expressing a number of milliseconds.
// NOTE: when ExpiryType is SetExpiryTypeNeverExpire, this value is ignored.
ExpiresOn string
jhendrixMSFT marked this conversation as resolved.
Show resolved Hide resolved

// ExpiryType indicates how the value of ExpiresOn should be interpreted (absolute, relative to now, etc).
ExpiryType SetExpiryType
}

func (e CreationExpiryTypeAbsolute) notPubliclyImplementable() {}

func (e CreationExpiryTypeRelativeToNow) Format() (generated.ExpiryOptions, *string) {
return generated.ExpiryOptionsRelativeToNow, to.Ptr(strconv.FormatInt(time.Duration(e).Milliseconds(), 10))
}

func (e CreationExpiryTypeRelativeToNow) notPubliclyImplementable() {}

func (e CreationExpiryTypeNever) Format() (generated.ExpiryOptions, *string) {
return generated.ExpiryOptionsNeverExpire, nil
}

func (e CreationExpiryTypeNever) notPubliclyImplementable() {}

// ACLFailedEntry contains the failed ACL entry (response model).
type ACLFailedEntry = path.ACLFailedEntry

// SetAccessControlRecursiveResponse contains part of the response data returned by the []OP_AccessControl operations.
type SetAccessControlRecursiveResponse = generated.SetAccessControlRecursiveResponse

// SetExpiryType defines values for ExpiryType.
type SetExpiryType = exported.SetExpiryType

// SetExpiryTypeAbsolute defines the absolute time for the expiry.
type SetExpiryTypeAbsolute = exported.SetExpiryTypeAbsolute

// SetExpiryTypeRelativeToNow defines the duration relative to now for the expiry.
type SetExpiryTypeRelativeToNow = exported.SetExpiryTypeRelativeToNow

// SetExpiryTypeRelativeToCreation defines the duration relative to creation for the expiry.
type SetExpiryTypeRelativeToCreation = exported.SetExpiryTypeRelativeToCreation

// SetExpiryTypeNever defines that will be set to never expire.
type SetExpiryTypeNever = exported.SetExpiryTypeNever

// SetExpiryOptions contains the optional parameters for the Client.SetExpiry method.
type SetExpiryOptions = exported.SetExpiryOptions
type SetExpiryOptions struct {
// placeholder for future options
}

type HTTPRange = exported.HTTPRange

Expand Down
71 changes: 0 additions & 71 deletions sdk/storage/azdatalake/internal/exported/set_expiry.go

This file was deleted.