describe time value standards, validate time flags (#1066)

## Description

Adds validation of cli inputs for time-based flags.
All time values that can be parsed by the time
handling in common count as supported, even
though official support includes a smaller set.

Also adds some clean-up to time.go and adds a
design document describing standard time value
formats, and the maintenance thereof, in corso.

## Type of change

- [x] 🌻 Feature
- [x] 🗺️ Documentation
- [x] 🤖 Test

## Issue(s)

* #943
* #924

## Test Plan

- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2022-10-06 11:46:21 -06:00 committed by GitHub
parent cffb00c44b
commit 89668ed164
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 263 additions and 109 deletions

25
design/time_standards.md Normal file
View File

@ -0,0 +1,25 @@
# Corso Standard Time Format
Since Corso primarily deals with with historical point-in-time events (created-at, delivered-at, started-on, etc), as opposed to scheduled future events, we can safely represent all time with point-in-time, timezoned values.
The standard string format uses [iso-8601](https://en.wikipedia.org/wiki/ISO_8601) date & time + [rfc-3339](https://datatracker.ietf.org/doc/html/rfc3339) compliant formats, with milliseconds. All time values should be stored in the UTC timezone.
ex:
* `2022-07-11T20:07:59.00000Z`
* `2022-07-11T20:07:59Z`
In golang implementation, use the time package format `time.RFC3339Nano`.
## Deviation From the Standard
The above standard helps ensure clean transformations when serializing to and from time.Time structs and strings. In certain cases time values will need to be displayed in a non-standard format. These variations are acceptable as long as the application has no intent to consume that format again in a future process.
Examples: the date-time suffix in restoration destination root folders or the human-readable format displayed in CLI outputs.
## Input Leniency
End users are not required to utilize the standard time format when calling inputs. In practice, all formats recognized in `/internal/common/time.go` can be used as a valid input value. Official format support is detailed in the public Corso Documentation.
## Maintenance
All supported time formats should appear as consts in `/internal/common/time.go`. Usage of `time.Parse()` and `time.Format()` should be kept to that package.

View File

@ -382,22 +382,6 @@ func detailsExchangeCmd(cmd *cobra.Command, args []string) error {
return nil
}
if err := utils.ValidateExchangeRestoreFlags(backupID); err != nil {
return err
}
s, acct, err := config.GetStorageAndAccount(ctx, true, nil)
if err != nil {
return Only(ctx, err)
}
r, err := repository.Connect(ctx, acct, s, options.Control())
if err != nil {
return Only(ctx, errors.Wrapf(err, "Failed to connect to the %s repository", s.Provider))
}
defer utils.CloseRepo(ctx, r)
opts := utils.ExchangeOpts{
Contacts: contact,
ContactFolders: contactFolder,
@ -418,6 +402,22 @@ func detailsExchangeCmd(cmd *cobra.Command, args []string) error {
EventSubject: eventSubject,
}
if err := utils.ValidateExchangeRestoreFlags(backupID, opts); err != nil {
return err
}
s, acct, err := config.GetStorageAndAccount(ctx, true, nil)
if err != nil {
return Only(ctx, err)
}
r, err := repository.Connect(ctx, acct, s, options.Control())
if err != nil {
return Only(ctx, errors.Wrapf(err, "Failed to connect to the %s repository", s.Provider))
}
defer utils.CloseRepo(ctx, r)
ds, err := runDetailsExchangeCmd(ctx, r, backupID, opts)
if err != nil {
return Only(ctx, err)

View File

@ -176,22 +176,6 @@ func restoreExchangeCmd(cmd *cobra.Command, args []string) error {
return nil
}
if err := utils.ValidateExchangeRestoreFlags(backupID); err != nil {
return err
}
s, a, err := config.GetStorageAndAccount(ctx, true, nil)
if err != nil {
return Only(ctx, err)
}
r, err := repository.Connect(ctx, a, s, options.Control())
if err != nil {
return Only(ctx, errors.Wrapf(err, "Failed to connect to the %s repository", s.Provider))
}
defer utils.CloseRepo(ctx, r)
opts := utils.ExchangeOpts{
Contacts: contact,
ContactFolders: contactFolder,
@ -212,6 +196,22 @@ func restoreExchangeCmd(cmd *cobra.Command, args []string) error {
EventSubject: eventSubject,
}
if err := utils.ValidateExchangeRestoreFlags(backupID, opts); err != nil {
return err
}
s, a, err := config.GetStorageAndAccount(ctx, true, nil)
if err != nil {
return Only(ctx, err)
}
r, err := repository.Connect(ctx, a, s, options.Control())
if err != nil {
return Only(ctx, errors.Wrapf(err, "Failed to connect to the %s repository", s.Provider))
}
defer utils.CloseRepo(ctx, r)
sel := selectors.NewExchangeRestore()
utils.IncludeExchangeRestoreDataSelectors(sel, opts)
utils.FilterExchangeRestoreInfoSelectors(sel, opts)
@ -221,7 +221,7 @@ func restoreExchangeCmd(cmd *cobra.Command, args []string) error {
sel.Include(sel.Users(selectors.Any()))
}
restoreDest := control.DefaultRestoreDestination(common.SimpleDateTimeFormat)
restoreDest := control.DefaultRestoreDestination(common.SimpleDateTime)
ro, err := r.NewRestore(ctx, backupID, sel.Selector, restoreDest)
if err != nil {

View File

@ -140,3 +140,35 @@ func (suite *RestoreExchangeIntegrationSuite) TestExchangeRestoreCmd() {
})
}
}
func (suite *RestoreExchangeIntegrationSuite) TestExchangeRestoreCmd_badTimeFlags() {
for _, set := range backupDataSets {
if set == contacts {
suite.T().Skip()
}
suite.T().Run(set.String(), func(t *testing.T) {
ctx := config.SetViper(tester.NewContext(), suite.vpr)
ctx, _ = logger.SeedLevel(ctx, logger.Development)
defer logger.Flush(ctx)
var timeFilter string
switch set {
case email:
timeFilter = "--email-received-after"
case events:
timeFilter = "--event-starts-after"
}
cmd := tester.StubRootCmd(
"restore", "exchange",
"--config-file", suite.cfgFP,
"--backup", string(suite.backupOps[set].Results.BackupID),
timeFilter, "smarf")
cli.BuildCommandTree(cmd)
// run the command
require.Error(t, cmd.ExecuteContext(ctx))
})
}
}

View File

@ -123,7 +123,17 @@ func restoreOneDriveCmd(cmd *cobra.Command, args []string) error {
return nil
}
if err := utils.ValidateOneDriveRestoreFlags(backupID); err != nil {
opts := utils.OneDriveOpts{
Users: user,
Paths: folderPaths,
Names: fileNames,
CreatedAfter: fileCreatedAfter,
CreatedBefore: fileCreatedBefore,
ModifiedAfter: fileModifiedAfter,
ModifiedBefore: fileModifiedBefore,
}
if err := utils.ValidateOneDriveRestoreFlags(backupID, opts); err != nil {
return err
}
@ -139,16 +149,6 @@ func restoreOneDriveCmd(cmd *cobra.Command, args []string) error {
defer utils.CloseRepo(ctx, r)
opts := utils.OneDriveOpts{
Users: user,
Paths: folderPaths,
Names: fileNames,
CreatedAfter: fileCreatedAfter,
CreatedBefore: fileCreatedBefore,
ModifiedAfter: fileModifiedAfter,
ModifiedBefore: fileModifiedBefore,
}
sel := selectors.NewOneDriveRestore()
utils.IncludeOneDriveRestoreDataSelectors(sel, opts)
utils.FilterOneDriveRestoreInfoSelectors(sel, opts)
@ -158,7 +158,7 @@ func restoreOneDriveCmd(cmd *cobra.Command, args []string) error {
sel.Include(sel.Users(selectors.Any()))
}
restoreDest := control.DefaultRestoreDestination(common.SimpleDateTimeFormatOneDrive)
restoreDest := control.DefaultRestoreDestination(common.SimpleDateTimeOneDrive)
ro, err := r.NewRestore(ctx, backupID, sel.Selector, restoreDest)
if err != nil {

View File

@ -72,11 +72,27 @@ func AddExchangeFilter(
}
// ValidateExchangeRestoreFlags checks common flags for correctness and interdependencies
func ValidateExchangeRestoreFlags(backupID string) error {
func ValidateExchangeRestoreFlags(backupID string, opts ExchangeOpts) error {
if len(backupID) == 0 {
return errors.New("a backup ID is required")
}
if !IsValidTimeFormat(opts.EmailReceivedAfter) {
return errors.New("invalid time format for email-received-after")
}
if !IsValidTimeFormat(opts.EmailReceivedBefore) {
return errors.New("invalid time format for email-received-before")
}
if !IsValidTimeFormat(opts.EventStartsAfter) {
return errors.New("invalid time format for event-starts-after")
}
if !IsValidTimeFormat(opts.EventStartsBefore) {
return errors.New("invalid time format for event-starts-before")
}
return nil
}

View File

@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/suite"
"github.com/alcionai/corso/src/cli/utils"
"github.com/alcionai/corso/src/internal/common"
"github.com/alcionai/corso/src/pkg/selectors"
)
@ -22,21 +23,35 @@ func (suite *ExchangeUtilsSuite) TestValidateBackupDetailFlags() {
table := []struct {
name string
backupID string
opts utils.ExchangeOpts
expect assert.ErrorAssertionFunc
}{
{
name: "with backupid",
backupID: "bid",
opts: utils.ExchangeOpts{},
expect: assert.NoError,
},
{
name: "no backupid",
opts: utils.ExchangeOpts{},
expect: assert.Error,
},
{
name: "valid time",
backupID: "bid",
opts: utils.ExchangeOpts{EmailReceivedAfter: common.Now()},
expect: assert.NoError,
},
{
name: "invalid time",
opts: utils.ExchangeOpts{EmailReceivedAfter: "fnords"},
expect: assert.Error,
},
}
for _, test := range table {
suite.T().Run(test.name, func(t *testing.T) {
test.expect(t, utils.ValidateExchangeRestoreFlags(test.backupID))
test.expect(t, utils.ValidateExchangeRestoreFlags(test.backupID, test.opts))
})
}
}

View File

@ -17,11 +17,27 @@ type OneDriveOpts struct {
}
// ValidateOneDriveRestoreFlags checks common flags for correctness and interdependencies
func ValidateOneDriveRestoreFlags(backupID string) error {
func ValidateOneDriveRestoreFlags(backupID string, opts OneDriveOpts) error {
if len(backupID) == 0 {
return errors.New("a backup ID is required")
}
if !IsValidTimeFormat(opts.CreatedAfter) {
return errors.New("invalid time format for created-after")
}
if !IsValidTimeFormat(opts.CreatedBefore) {
return errors.New("invalid time format for created-before")
}
if !IsValidTimeFormat(opts.ModifiedAfter) {
return errors.New("invalid time format for modified-after")
}
if !IsValidTimeFormat(opts.ModifiedBefore) {
return errors.New("invalid time format for modified-before")
}
return nil
}

View File

@ -8,6 +8,7 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/alcionai/corso/src/internal/common"
"github.com/alcionai/corso/src/pkg/repository"
)
@ -57,3 +58,20 @@ func AddCommand(parent, c *cobra.Command) (*cobra.Command, *pflag.FlagSet) {
return c, c.Flags()
}
// IsValidTimeFormat returns true if the input is regonized as a
// supported format by the common time parser. Returns true if
// the input is zero valued, which indicates that the flag was not
// called.
func IsValidTimeFormat(in string) bool {
if len(in) == 0 {
return true
}
_, err := common.ParseTime(in)
if err != nil {
return false
}
return true
}

View File

@ -7,26 +7,47 @@ import (
"github.com/pkg/errors"
)
type TimeFormat string
const (
// the clipped format occurs when m365 removes the :00 second suffix
ClippedSimpleTimeFormat = "02-Jan-2006_15:04"
ClippedSimpleTimeFormatOneDrive = "02-Jan-2006_15-04"
LegacyTimeFormat = time.RFC3339
SimpleDateTimeFormat = "02-Jan-2006_15:04:05"
// SimpleDateTimeFormatOneDrive is similar to `SimpleDateTimeFormat`
// but uses `-` instead of `:` which is a reserved character in
// OneDrive
SimpleDateTimeFormatOneDrive = "02-Jan-2006_15-04-05"
StandardTimeFormat = time.RFC3339Nano
TabularOutputTimeFormat = "2006-01-02T15:04:05Z"
// Format used for test restore destination folders. Microsecond granularity.
SimpleDateTimeFormatTests = SimpleDateTimeFormatOneDrive + ".000000"
// StandardTime is the canonical format used for all data storage in corso
StandardTime TimeFormat = time.RFC3339Nano
// DateOnly is accepted by the CLI as a valid input for timestamp-based
// filters. Time and timezone are assumed to be 00:00:00 and UTC.
DateOnly TimeFormat = "2006-01-02"
// TabularOutput is used when displaying time values to the user in
// non-json cli outputs.
TabularOutput TimeFormat = "2006-01-02T15:04:05Z"
// LegacyTime is used in /exchange/service_restore to comply with certain
// graphAPI time format requirements.
LegacyTime TimeFormat = time.RFC3339
// SimpleDateTime is the default value appended to the root restoration folder name.
SimpleDateTime TimeFormat = "02-Jan-2006_15:04:05"
// SimpleDateTimeOneDrive modifies SimpleDateTimeFormat to comply with onedrive folder
// restrictions: primarily swapping `-` instead of `:` which is a reserved character.
SimpleDateTimeOneDrive TimeFormat = "02-Jan-2006_15-04-05"
// m365 will remove the :00 second suffix on folder names, resulting in the following formats.
ClippedSimple TimeFormat = "02-Jan-2006_15:04"
ClippedSimpleOneDrive TimeFormat = "02-Jan-2006_15-04"
// SimpleTimeTesting is used for testing restore destination folders.
// Microsecond granularity prevents collisions in parallel package or workflow runs.
SimpleTimeTesting TimeFormat = SimpleDateTimeOneDrive + ".000000"
)
// these regexes are used to extract time formats from strings. Their primary purpose is to
// identify the folders produced in external data during automated testing. For safety, each
// time format described above should have a matching regexp.
var (
clippedSimpleTimeRE = regexp.MustCompile(`.*(\d{2}-[a-zA-Z]{3}-\d{4}_\d{2}:\d{2}).*`)
clippedSimpleTimeOneDriveRE = regexp.MustCompile(`.*(\d{2}-[a-zA-Z]{3}-\d{4}_\d{2}-\d{2}).*`)
legacyTimeRE = regexp.MustCompile(
clippedSimpleRE = regexp.MustCompile(`.*(\d{2}-[a-zA-Z]{3}-\d{4}_\d{2}:\d{2}).*`)
clippedSimpleOneDriveRE = regexp.MustCompile(`.*(\d{2}-[a-zA-Z]{3}-\d{4}_\d{2}-\d{2}).*`)
dateOnlyRE = regexp.MustCompile(`.*(\d{4}-\d{2}-\d{2}).*`)
legacyTimeRE = regexp.MustCompile(
`.*(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}?([Zz]|[a-zA-Z]{2}|([\+|\-]([01]\d|2[0-3])))).*`)
simpleDateTimeRE = regexp.MustCompile(`.*(\d{2}-[a-zA-Z]{3}-\d{4}_\d{2}:\d{2}:\d{2}).*`)
simpleDateTimeOneDriveRE = regexp.MustCompile(`.*(\d{2}-[a-zA-Z]{3}-\d{4}_\d{2}-\d{2}-\d{2}).*`)
@ -36,15 +57,17 @@ var (
)
var (
// clipped formats must appear last, else they take priority over the regular Simple format.
formats = []string{
StandardTimeFormat,
SimpleDateTimeFormat,
SimpleDateTimeFormatOneDrive,
LegacyTimeFormat,
TabularOutputTimeFormat,
ClippedSimpleTimeFormat,
ClippedSimpleTimeFormatOneDrive,
// shortened formats (clipped*, DateOnly) must follow behind longer formats, otherwise they'll
// get eagerly chosen as the parsable format, slicing out some data.
formats = []TimeFormat{
StandardTime,
SimpleDateTime,
SimpleDateTimeOneDrive,
LegacyTime,
TabularOutput,
ClippedSimple,
ClippedSimpleOneDrive,
DateOnly,
}
regexes = []*regexp.Regexp{
standardTimeRE,
@ -52,47 +75,53 @@ var (
simpleDateTimeOneDriveRE,
legacyTimeRE,
tabularOutputTimeRE,
clippedSimpleTimeRE,
clippedSimpleTimeOneDriveRE,
clippedSimpleRE,
clippedSimpleOneDriveRE,
dateOnlyRE,
}
)
var ErrNoTimeString = errors.New("no substring contains a known time format")
// Now produces the current time as a string in the standard format.
func Now() string {
return FormatNow(StandardTime)
}
// FormatNow produces the current time in UTC using the provided
// time format.
func FormatNow(fmt string) string {
return time.Now().UTC().Format(fmt)
func FormatNow(fmt TimeFormat) string {
return FormatTimeWith(time.Now(), fmt)
}
// FormatTimeWith produces the a datetime with the given format.
func FormatTimeWith(t time.Time, fmt string) string {
return t.UTC().Format(fmt)
func FormatTimeWith(t time.Time, fmt TimeFormat) string {
return t.UTC().Format(string(fmt))
}
// FormatTime produces the standard format for corso time values.
// Always formats into the UTC timezone.
func FormatTime(t time.Time) string {
return FormatTimeWith(t, StandardTimeFormat)
return FormatTimeWith(t, StandardTime)
}
// FormatSimpleDateTime produces a simple datetime of the format
// "02-Jan-2006_15:04:05"
func FormatSimpleDateTime(t time.Time) string {
return FormatTimeWith(t, SimpleDateTimeFormat)
return FormatTimeWith(t, SimpleDateTime)
}
// FormatTabularDisplayTime produces the standard format for displaying
// a timestamp as part of user-readable cli output.
// "2016-01-02T15:04:05Z"
func FormatTabularDisplayTime(t time.Time) string {
return FormatTimeWith(t, TabularOutputTimeFormat)
return FormatTimeWith(t, TabularOutput)
}
// FormatLegacyTime produces standard format for string values
// that are placed in SingleValueExtendedProperty tags
func FormatLegacyTime(t time.Time) string {
return FormatTimeWith(t, LegacyTimeFormat)
return FormatTimeWith(t, LegacyTime)
}
// ParseTime makes a best attempt to produce a time value from
@ -103,7 +132,7 @@ func ParseTime(s string) (time.Time, error) {
}
for _, form := range formats {
t, err := time.Parse(form, s)
t, err := time.Parse(string(form), s)
if err == nil {
return t.UTC(), nil
}

View File

@ -37,7 +37,7 @@ func (suite *CommonTimeUnitSuite) TestFormatTabularDisplayTime() {
t := suite.T()
now := time.Now()
result := common.FormatTabularDisplayTime(now)
assert.Equal(t, now.UTC().Format(common.TabularOutputTimeFormat), result)
assert.Equal(t, now.UTC().Format(string(common.TabularOutput)), result)
}
func (suite *CommonTimeUnitSuite) TestParseTime() {
@ -57,11 +57,11 @@ func (suite *CommonTimeUnitSuite) TestParseTime() {
}
func (suite *CommonTimeUnitSuite) TestExtractTime() {
comparable := func(t *testing.T, tt time.Time, clippedFormat string) time.Time {
comparable := func(t *testing.T, tt time.Time, shortFormat common.TimeFormat) time.Time {
ts := common.FormatLegacyTime(tt.UTC())
if len(clippedFormat) > 0 {
ts = tt.UTC().Format(clippedFormat)
if len(shortFormat) > 0 {
ts = tt.UTC().Format(string(shortFormat))
}
c, err := common.ParseTime(ts)
@ -91,15 +91,16 @@ func (suite *CommonTimeUnitSuite) TestExtractTime() {
type timeFormatter func(time.Time) string
formats := []string{
common.ClippedSimpleTimeFormat,
common.ClippedSimpleTimeFormatOneDrive,
common.LegacyTimeFormat,
common.SimpleDateTimeFormat,
common.SimpleDateTimeFormatOneDrive,
common.StandardTimeFormat,
common.TabularOutputTimeFormat,
common.SimpleDateTimeFormatTests,
formats := []common.TimeFormat{
common.ClippedSimple,
common.ClippedSimpleOneDrive,
common.LegacyTime,
common.SimpleDateTime,
common.SimpleDateTimeOneDrive,
common.StandardTime,
common.TabularOutput,
common.SimpleTimeTesting,
common.DateOnly,
}
type presuf struct {
@ -116,7 +117,7 @@ func (suite *CommonTimeUnitSuite) TestExtractTime() {
type testable struct {
input string
clippedFormat string
clippedFormat common.TimeFormat
expect time.Time
}
@ -125,10 +126,12 @@ func (suite *CommonTimeUnitSuite) TestExtractTime() {
// test matrix: for each input, in each format, with each prefix/suffix, run the test.
for _, in := range inputs {
for _, f := range formats {
clippedFormat := f
shortFormat := f
if f != common.ClippedSimpleTimeFormat && f != common.ClippedSimpleTimeFormatOneDrive {
clippedFormat = ""
if f != common.ClippedSimple &&
f != common.ClippedSimpleOneDrive &&
f != common.DateOnly {
shortFormat = ""
}
v := common.FormatTimeWith(in, f)
@ -136,8 +139,8 @@ func (suite *CommonTimeUnitSuite) TestExtractTime() {
for _, ps := range pss {
table = append(table, testable{
input: ps.prefix + v + ps.suffix,
expect: comparable(suite.T(), in, clippedFormat),
clippedFormat: clippedFormat,
expect: comparable(suite.T(), in, shortFormat),
clippedFormat: shortFormat,
})
}
}

View File

@ -27,7 +27,7 @@ func TestEventSuite(t *testing.T) {
func (suite *EventSuite) TestEventInfo() {
initial := time.Now()
now := initial.Format(common.StandardTimeFormat)
now := common.FormatTime(initial)
suite.T().Logf("Initial: %v\nFormatted: %v\n", initial, now)
tests := []struct {
@ -48,7 +48,7 @@ func (suite *EventSuite) TestEventInfo() {
dateTime := models.NewDateTimeTimeZone()
dateTime.SetDateTime(&now)
event.SetStart(dateTime)
full, err := time.Parse(common.StandardTimeFormat, now)
full, err := common.ParseTime(now)
require.NoError(suite.T(), err)
i := &details.ExchangeInfo{
ItemType: details.ExchangeEvent,

View File

@ -65,7 +65,7 @@ const (
// Contents verified as working with sample data from kiota-serialization-json-go v0.5.5
func GetMockMessageBytes(subject string) []byte {
userID := "foobar@8qzvrj.onmicrosoft.com"
timestamp := " " + common.FormatNow(common.SimpleDateTimeFormat)
timestamp := " " + common.FormatNow(common.SimpleDateTime)
message := fmt.Sprintf(
messageTmpl,

View File

@ -7,5 +7,5 @@ import (
func DefaultTestRestoreDestination() control.RestoreDestination {
// Use microsecond granularity to help reduce collisions.
return control.DefaultRestoreDestination(common.SimpleDateTimeFormatTests)
return control.DefaultRestoreDestination(common.SimpleTimeTesting)
}

View File

@ -24,7 +24,7 @@ func TestDetailsUnitSuite(t *testing.T) {
}
func (suite *DetailsUnitSuite) TestDetailsEntry_HeadersValues() {
nowStr := common.FormatNow(common.TabularOutputTimeFormat)
nowStr := common.FormatNow(common.TabularOutput)
now, err := common.ParseTime(nowStr)
require.NoError(suite.T(), err)

View File

@ -46,7 +46,7 @@ type RestoreDestination struct {
ContainerName string
}
func DefaultRestoreDestination(timeFormat string) RestoreDestination {
func DefaultRestoreDestination(timeFormat common.TimeFormat) RestoreDestination {
return RestoreDestination{
ContainerName: defaultRestoreLocation + common.FormatNow(timeFormat),
}