Skip to content

Commit

Permalink
feat: allow using UUID prefix as argument
Browse files Browse the repository at this point in the history
Also refactors argument resolution to be more flexible. This allows later adding support for case-insensitive and wild-card matching.
  • Loading branch information
kangasta committed Nov 14, 2024
1 parent 655bbda commit 65ccd95
Show file tree
Hide file tree
Showing 31 changed files with 332 additions and 299 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- Take server state into account in server completions. For example, do not offer started servers as completions for `server start` command.
- Allow using UUID prefix as an argument. For example, if there is only one network available that has an UUID starting with `0316`, details of that network can be listed with `upctl network show 0316` command.

## [3.11.1] - 2024-08-12

Expand Down
3 changes: 2 additions & 1 deletion internal/commands/network/modify.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ func (s *modifyCommand) ExecuteSingleArgument(exec commands.Executor, arg string
if err != nil {
return commands.HandleError(exec, msg, fmt.Errorf("cannot get router resolver: %w", err))
}
routerUUID, err := routerResolver(s.attachRouter)
resolved := routerResolver(s.attachRouter)
routerUUID, err := resolved.GetOnly()
if err != nil {
return commands.HandleError(exec, msg, fmt.Errorf("cannot resolve router '%s': %w", s.attachRouter, err))
}
Expand Down
5 changes: 3 additions & 2 deletions internal/commands/runcommand.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@ func resolveArguments(nc Command, exec Executor, args []string) (out []resolvedA
return nil, fmt.Errorf("cannot get resolver: %w", err)
}
for _, arg := range args {
resolved, err := argumentResolver(arg)
out = append(out, resolvedArgument{Resolved: resolved, Error: err, Original: arg})
resolved := argumentResolver(arg)
value, err := resolved.GetOnly()
out = append(out, resolvedArgument{Resolved: value, Error: err, Original: arg})
}
} else {
for _, arg := range args {
Expand Down
11 changes: 6 additions & 5 deletions internal/commands/runcommand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,12 @@ type mockMultiResolver struct {
}

func (m *mockMultiResolver) Get(_ context.Context, _ internal.AllServices) (resolver.Resolver, error) {
return func(arg string) (uuid string, err error) {
if len(arg) > 5 {
return "", fmt.Errorf("MOCKTOOLONG")
return func(arg string) resolver.Resolved {
rv := resolver.Resolved{Arg: arg}
if len(arg) <= 5 {
rv.AddMatch("uuid:"+arg, resolver.MatchTypeExact)
}
return fmt.Sprintf("uuid:%s", arg), nil
return rv
}, nil
}

Expand Down Expand Up @@ -258,7 +259,7 @@ func TestExecute_Resolution(t *testing.T) {
values[typedO.Value.(string)] = struct{}{}
case output.Error:
assert.Empty(t, typedO.Resolved)
assert.EqualError(t, typedO.Value, "cannot resolve argument: MOCKTOOLONG")
assert.EqualError(t, typedO.Value, "cannot resolve argument: nothing found matching 'failtoresolve'")
}
}
assert.Equal(t, values, map[string]struct{}{
Expand Down
3 changes: 2 additions & 1 deletion internal/namedargs/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ func Resolve(provider resolver.ResolutionProvider, exec commands.Executor, arg s
return "", fmt.Errorf("could not initialize resolver: %w", err)
}

return resolver(arg)
resolved := resolver(arg)
return resolved.GetOnly()
}
16 changes: 4 additions & 12 deletions internal/resolver/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,12 @@ func (s CachingAccount) Get(ctx context.Context, svc internal.AllServices) (Reso
if err != nil {
return nil, err
}
return func(arg string) (uuid string, err error) {
rv := ""
return func(arg string) Resolved {
rv := Resolved{Arg: arg}
for _, account := range accounts {
if MatchArgWithWhitespace(arg, account.Username) {
if rv != "" {
return "", AmbiguousResolutionError(arg)
}
rv = account.Username
}
rv.AddMatch(account.Username, MatchArgWithWhitespace(arg, account.Username))
}
if rv != "" {
return rv, nil
}
return "", NotFoundError(arg)
return rv
}, nil
}

Expand Down
11 changes: 7 additions & 4 deletions internal/resolver/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ func TestAccountResolution(t *testing.T) {
argResolver, err := res.Get(context.TODO(), mService)
assert.NoError(t, err)
for _, account := range allAccounts {
resolved, err := argResolver(account.Username)
resolved := argResolver(account.Username)
value, err := resolved.GetOnly()
assert.NoError(t, err)
assert.Equal(t, account.Username, resolved)
assert.Equal(t, account.Username, value)
}
// make sure caching works, eg. we didn't call GetAccountList more than once
mService.AssertNumberOfCalls(t, "GetAccountList", 1)
Expand All @@ -60,12 +61,14 @@ func TestAccountResolution(t *testing.T) {
assert.NoError(t, err)

// not found
resolved, err := argResolver("notfound")
resolved := argResolver("notfound")
value, err := resolved.GetOnly()

if !assert.Error(t, err) {
t.FailNow()
}
assert.ErrorIs(t, err, resolver.NotFoundError("notfound"))
assert.Equal(t, "", resolved)
assert.Equal(t, "", value)

// make sure caching works, eg. we didn't call GetAccountList more than once
mService.AssertNumberOfCalls(t, "GetAccountList", 1)
Expand Down
17 changes: 5 additions & 12 deletions internal/resolver/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,13 @@ func (s CachingDatabase) Get(ctx context.Context, svc internal.AllServices) (Res
if err != nil {
return nil, err
}
return func(arg string) (uuid string, err error) {
rv := ""
return func(arg string) Resolved {
rv := Resolved{Arg: arg}
for _, db := range databases {
if MatchArgWithWhitespace(arg, db.Title) || db.UUID == arg {
if rv != "" {
return "", AmbiguousResolutionError(arg)
}
rv = db.UUID
}
rv.AddMatch(db.UUID, MatchArgWithWhitespace(arg, db.Title))
rv.AddMatch(db.UUID, MatchUUID(arg, db.UUID))
}
if rv != "" {
return rv, nil
}
return "", NotFoundError(arg)
return rv
}, nil
}

Expand Down
22 changes: 13 additions & 9 deletions internal/resolver/database_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ func TestDatabaseResolution(t *testing.T) {
argResolver, err := res.Get(context.TODO(), mService)
assert.NoError(t, err)
for _, db := range mockDatabases {
resolved, err := argResolver(db.UUID)
resolved := argResolver(db.UUID)
value, err := resolved.GetOnly()
assert.NoError(t, err)
assert.Equal(t, db.UUID, resolved)
assert.Equal(t, db.UUID, value)
}

// Make sure caching works, eg. we didn't call GetManagedDatabases more than once
Expand All @@ -44,9 +45,10 @@ func TestDatabaseResolution(t *testing.T) {
assert.NoError(t, err)

db := mockDatabases[2]
resolved, err := argResolver(db.Title)
resolved := argResolver(db.Title)
value, err := resolved.GetOnly()
assert.NoError(t, err)
assert.Equal(t, db.UUID, resolved)
assert.Equal(t, db.UUID, value)
// Make sure caching works, eg. we didn't call GetManagedDatabases more than once
mService.AssertNumberOfCalls(t, "GetManagedDatabases", 1)
})
Expand All @@ -58,23 +60,25 @@ func TestDatabaseResolution(t *testing.T) {
res := resolver.CachingDatabase{}
argResolver, err := res.Get(context.TODO(), mService)
assert.NoError(t, err)
var resolved string

// Ambiguous title
resolved, err = argResolver("asd")
resolved := argResolver("asd")
value, err := resolved.GetOnly()
if !assert.Error(t, err) {
t.FailNow()
}
assert.ErrorIs(t, err, resolver.AmbiguousResolutionError("asd"))
assert.Equal(t, "", resolved)
assert.Equal(t, "", value)

// Not found
resolved, err = argResolver("not-found")
resolved = argResolver("not-found")
value, err = resolved.GetOnly()

if !assert.Error(t, err) {
t.FailNow()
}
assert.ErrorIs(t, err, resolver.NotFoundError("not-found"))
assert.Equal(t, "", resolved)
assert.Equal(t, "", value)

// Make sure caching works, eg. we didn't call GetManagedDatabases more than once
mService.AssertNumberOfCalls(t, "GetManagedDatabases", 1)
Expand Down
17 changes: 5 additions & 12 deletions internal/resolver/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,13 @@ func (s CachingGateway) Get(ctx context.Context, svc internal.AllServices) (Reso
if err != nil {
return nil, err
}
return func(arg string) (uuid string, err error) {
rv := ""
return func(arg string) Resolved {
rv := Resolved{Arg: arg}
for _, gtw := range gateways {
if MatchArgWithWhitespace(arg, gtw.Name) || gtw.UUID == arg {
if rv != "" {
return "", AmbiguousResolutionError(arg)
}
rv = gtw.UUID
}
rv.AddMatch(gtw.UUID, MatchArgWithWhitespace(arg, gtw.Name))
rv.AddMatch(gtw.UUID, MatchUUID(arg, gtw.UUID))
}
if rv != "" {
return rv, nil
}
return "", NotFoundError(arg)
return rv
}, nil
}

Expand Down
21 changes: 12 additions & 9 deletions internal/resolver/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ func TestGatewayResolution(t *testing.T) {
argResolver, err := res.Get(context.TODO(), mService)
assert.NoError(t, err)
for _, db := range mockGateways {
resolved, err := argResolver(db.UUID)
resolved := argResolver(db.UUID)
value, err := resolved.GetOnly()
assert.NoError(t, err)
assert.Equal(t, db.UUID, resolved)
assert.Equal(t, db.UUID, value)
}

// Make sure caching works, eg. we didn't call GetGateways more than once
Expand All @@ -44,9 +45,10 @@ func TestGatewayResolution(t *testing.T) {
assert.NoError(t, err)

db := mockGateways[2]
resolved, err := argResolver(db.Name)
resolved := argResolver(db.Name)
value, err := resolved.GetOnly()
assert.NoError(t, err)
assert.Equal(t, db.UUID, resolved)
assert.Equal(t, db.UUID, value)
// Make sure caching works, eg. we didn't call GetGateways more than once
mService.AssertNumberOfCalls(t, "GetGateways", 1)
})
Expand All @@ -58,23 +60,24 @@ func TestGatewayResolution(t *testing.T) {
res := resolver.CachingGateway{}
argResolver, err := res.Get(context.TODO(), mService)
assert.NoError(t, err)
var resolved string

// Ambiguous Name
resolved, err = argResolver("asd")
resolved := argResolver("asd")
value, err := resolved.GetOnly()
if !assert.Error(t, err) {
t.FailNow()
}
assert.ErrorIs(t, err, resolver.AmbiguousResolutionError("asd"))
assert.Equal(t, "", resolved)
assert.Equal(t, "", value)

// Not found
resolved, err = argResolver("not-found")
resolved = argResolver("not-found")
value, err = resolved.GetOnly()
if !assert.Error(t, err) {
t.FailNow()
}
assert.ErrorIs(t, err, resolver.NotFoundError("not-found"))
assert.Equal(t, "", resolved)
assert.Equal(t, "", value)

// Make sure caching works, eg. we didn't call GetGateways more than once
mService.AssertNumberOfCalls(t, "GetGateways", 1)
Expand Down
17 changes: 5 additions & 12 deletions internal/resolver/ipaddress.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,13 @@ func (s CachingIPAddress) Get(ctx context.Context, svc internal.AllServices) (Re
if err != nil {
return nil, err
}
return func(arg string) (uuid string, err error) {
rv := ""
return func(arg string) Resolved {
rv := Resolved{Arg: arg}
for _, ipAddress := range ipaddresses.IPAddresses {
if ipAddress.PTRRecord == arg || ipAddress.Address == arg {
if rv != "" {
return "", AmbiguousResolutionError(arg)
}
rv = ipAddress.Address
}
rv.AddMatch(ipAddress.Address, MatchArgWithWhitespace(arg, ipAddress.PTRRecord))
rv.AddMatch(ipAddress.Address, MatchArgWithWhitespace(arg, ipAddress.Address))
}
if rv != "" {
return rv, nil
}
return "", NotFoundError(arg)
return rv
}, nil
}

Expand Down
41 changes: 13 additions & 28 deletions internal/resolver/ipaddress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,6 @@ func TestIPAddressResolution(t *testing.T) {
Zone: "fi-hel1",
}
ipAddress5 := upcloud.IPAddress{
Address: "94.237.117.154", // same IP as 4 (not sure if this is actually possible?)
Access: "public",
Family: "IPv4",
PartOfPlan: upcloud.FromBool(true),
PTRRecord: "94-237-117-155.fi-hel1.upcloud.host",
ServerUUID: "005ab220-7ff6-42c9-8615-e4c02eb4104e",
MAC: "ee:1b:db:ca:6b:84",
Floating: upcloud.FromBool(false),
Zone: "fi-hel1",
}
ipAddress6 := upcloud.IPAddress{
Address: "94.237.117.156",
Access: "public",
Family: "IPv4",
Expand All @@ -81,7 +70,7 @@ func TestIPAddressResolution(t *testing.T) {
}

addresses := &upcloud.IPAddresses{IPAddresses: []upcloud.IPAddress{
ipAddress1, ipAddress2, ipAddress3, ipAddress4, ipAddress5, ipAddress6,
ipAddress1, ipAddress2, ipAddress3, ipAddress4, ipAddress5,
}}
unambiguousAddresses := []upcloud.IPAddress{ipAddress1, ipAddress2, ipAddress3}

Expand All @@ -92,9 +81,10 @@ func TestIPAddressResolution(t *testing.T) {
argResolver, err := res.Get(context.TODO(), mService)
assert.NoError(t, err)
for _, network := range unambiguousAddresses {
resolved, err := argResolver(network.Address)
resolved := argResolver(network.Address)
value, err := resolved.GetOnly()
assert.NoError(t, err)
assert.Equal(t, network.Address, resolved)
assert.Equal(t, network.Address, value)
}
// make sure caching works, eg. we didn't call GetServers more than once
mService.AssertNumberOfCalls(t, "GetIPAddresses", 1)
Expand All @@ -107,9 +97,10 @@ func TestIPAddressResolution(t *testing.T) {
argResolver, err := res.Get(context.TODO(), mService)
assert.NoError(t, err)
for _, network := range unambiguousAddresses {
resolved, err := argResolver(network.PTRRecord)
resolved := argResolver(network.PTRRecord)
value, err := resolved.GetOnly()
assert.NoError(t, err)
assert.Equal(t, network.Address, resolved)
assert.Equal(t, network.Address, value)
}
// make sure caching works, eg. we didn't call GetServers more than once
mService.AssertNumberOfCalls(t, "GetIPAddresses", 1)
Expand All @@ -122,29 +113,23 @@ func TestIPAddressResolution(t *testing.T) {
argResolver, err := res.Get(context.TODO(), mService)
assert.NoError(t, err)

// ambiguous address
resolved, err := argResolver(ipAddress4.Address)
if !assert.Error(t, err) {
t.FailNow()
}
assert.ErrorIs(t, err, resolver.AmbiguousResolutionError(ipAddress4.Address))
assert.Equal(t, "", resolved)

// ambiguous ptr record
resolved, err = argResolver(ipAddress4.PTRRecord)
resolved := argResolver(ipAddress4.PTRRecord)
value, err := resolved.GetOnly()
if !assert.Error(t, err) {
t.FailNow()
}
assert.ErrorIs(t, err, resolver.AmbiguousResolutionError(ipAddress4.PTRRecord))
assert.Equal(t, "", resolved)
assert.Equal(t, "", value)

// not found
resolved, err = argResolver("notfound")
resolved = argResolver("notfound")
value, err = resolved.GetOnly()
if !assert.Error(t, err) {
t.FailNow()
}
assert.ErrorIs(t, err, resolver.NotFoundError("notfound"))
assert.Equal(t, "", resolved)
assert.Equal(t, "", value)

// make sure caching works, eg. we didn't call GetServers more than once
mService.AssertNumberOfCalls(t, "GetIPAddresses", 1)
Expand Down
Loading

0 comments on commit 65ccd95

Please sign in to comment.