Skip to content

Commit

Permalink
feat: allow using UUID prefix as argument (#341)
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 authored Nov 15, 2024
1 parent 655bbda commit 27ed056
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 27ed056

Please sign in to comment.