From f831376c413f9213482014cb07cb6751ff22e22d Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 15 Jun 2021 18:03:43 +0200 Subject: [PATCH] Refactor static registry --- .../grpc/services/appregistry/appregistry.go | 5 +- .../services/appregistry/appregistry_test.go | 102 ++++++++++------ pkg/app/app.go | 5 +- pkg/app/registry/static/static.go | 113 ++++++++++++++---- 4 files changed, 165 insertions(+), 60 deletions(-) diff --git a/internal/grpc/services/appregistry/appregistry.go b/internal/grpc/services/appregistry/appregistry.go index 74449f183b..419cb03757 100644 --- a/internal/grpc/services/appregistry/appregistry.go +++ b/internal/grpc/services/appregistry/appregistry.go @@ -88,6 +88,7 @@ func parseConfig(m map[string]interface{}) (*config, error) { if err := mapstructure.Decode(m, c); err != nil { return nil, err } + c.init() return c, nil } @@ -99,7 +100,7 @@ func getRegistry(c *config) (app.Registry, error) { } func (s *svc) GetAppProviders(ctx context.Context, req *registrypb.GetAppProvidersRequest) (*registrypb.GetAppProvidersResponse, error) { - p, err := s.reg.FindProvider(ctx, req.ResourceInfo.MimeType) + p, err := s.reg.FindProviders(ctx, req.ResourceInfo.MimeType) if err != nil { return ®istrypb.GetAppProvidersResponse{ Status: status.NewInternal(ctx, err, "error looking for the app provider"), @@ -108,7 +109,7 @@ func (s *svc) GetAppProviders(ctx context.Context, req *registrypb.GetAppProvide res := ®istrypb.GetAppProvidersResponse{ Status: status.NewOK(ctx), - Providers: []*registrypb.ProviderInfo{p}, + Providers: p, } return res, nil } diff --git a/internal/grpc/services/appregistry/appregistry_test.go b/internal/grpc/services/appregistry/appregistry_test.go index 158e1e282d..684a1c3ce6 100644 --- a/internal/grpc/services/appregistry/appregistry_test.go +++ b/internal/grpc/services/appregistry/appregistry_test.go @@ -38,15 +38,21 @@ func (a ByAddress) Swap(i, j int) { a[i], a[j] = a[j], a[i] } func Test_ListAppProviders(t *testing.T) { tests := []struct { - name string - rules map[string]interface{} - want *registrypb.ListAppProvidersResponse + name string + providers map[string]interface{} + want *registrypb.ListAppProvidersResponse }{ { name: "simple test", - rules: map[string]interface{}{ - "text/json": "some Address", - "currently/ignored": "an other address", + providers: map[string]interface{}{ + "some Address": map[string]interface{}{ + "address": "some Address", + "mimetypes": []string{"text/json"}, + }, + "another address": map[string]interface{}{ + "address": "another address", + "mimetypes": []string{"currently/ignored"}, + }, }, // only Status and Providers will be asserted in the tests @@ -57,27 +63,36 @@ func Test_ListAppProviders(t *testing.T) { Message: "", }, Providers: []*registrypb.ProviderInfo{ - {Address: "some Address"}, - {Address: "an other address"}, + { + Address: "some Address", + MimeTypes: []string{"text/json"}, + }, + { + Address: "another address", + MimeTypes: []string{"currently/ignored"}, + }, }, }, }, { - name: "rules is nil", - rules: nil, + name: "providers is nil", + providers: nil, want: ®istrypb.ListAppProvidersResponse{ Status: &rpcv1beta1.Status{ Code: 1, Trace: "00000000000000000000000000000000", }, Providers: []*registrypb.ProviderInfo{ - {Address: ""}, + { + Address: "", + MimeTypes: []string{"text/plain"}, + }, }, }, }, { - name: "empty rules", - rules: map[string]interface{}{}, + name: "empty providers", + providers: map[string]interface{}{}, // only Status and Providers will be asserted in the tests want: ®istrypb.ListAppProvidersResponse{ @@ -87,14 +102,17 @@ func Test_ListAppProviders(t *testing.T) { Message: "", }, Providers: []*registrypb.ProviderInfo{ - {Address: ""}, + { + Address: "", + MimeTypes: []string{"text/plain"}, + }, }, }, }, { - name: "rule value is nil", - rules: map[string]interface{}{ - "text/json": nil, + name: "provider value is nil", + providers: map[string]interface{}{ + "some Address": nil, }, // only Status and Providers will be asserted in the tests @@ -104,16 +122,14 @@ func Test_ListAppProviders(t *testing.T) { Trace: "00000000000000000000000000000000", Message: "", }, - Providers: []*registrypb.ProviderInfo{ - {Address: ""}, - }, + Providers: []*registrypb.ProviderInfo{nil}, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rr, err := static.New(map[string]interface{}{"Rules": tt.rules}) + rr, err := static.New(map[string]interface{}{"Providers": tt.providers}) if err != nil { t.Errorf("could not create registry error = %v", err) return @@ -137,13 +153,19 @@ func Test_ListAppProviders(t *testing.T) { } func Test_GetAppProviders(t *testing.T) { - rules := map[string]interface{}{ - "text/json": "JSON format", - "image/bmp": "Windows OS/2 Bitmap Graphics", - "application/vnd.openxmlformats-officedocument.wordprocessingml.document": "Microsoft Word (OpenXML)", - "application/vnd.oasis.opendocument.presentation": "OpenDocument presentation document", - "application/vnd.apple.installer+xml": "Apple Installer Package", - "text/xml": "XML", + providers := map[string]interface{}{ + "text appprovider addr": map[string]interface{}{ + "address": "text appprovider addr", + "mimetypes": []string{"text/json", "text/xml"}, + }, + "image appprovider addr": map[string]interface{}{ + "address": "image appprovider addr", + "mimetypes": []string{"image/bmp"}, + }, + "misc appprovider addr": map[string]interface{}{ + "address": "misc appprovider addr", + "mimetypes": []string{"application/vnd.openxmlformats-officedocument.wordprocessingml.document", "application/vnd.oasis.opendocument.presentation", "application/vnd.apple.installer+xml"}, + }, } tests := []struct { @@ -162,7 +184,10 @@ func Test_GetAppProviders(t *testing.T) { Message: "", }, Providers: []*registrypb.ProviderInfo{ - {Address: "JSON format"}, + { + Address: "text appprovider addr", + MimeTypes: []string{"text/json", "text/xml"}, + }, }, }, }, @@ -176,7 +201,10 @@ func Test_GetAppProviders(t *testing.T) { Message: "", }, Providers: []*registrypb.ProviderInfo{ - {Address: "Apple Installer Package"}, + { + Address: "misc appprovider addr", + MimeTypes: []string{"application/vnd.openxmlformats-officedocument.wordprocessingml.document", "application/vnd.oasis.opendocument.presentation", "application/vnd.apple.installer+xml"}, + }, }, }, }, @@ -230,7 +258,7 @@ func Test_GetAppProviders(t *testing.T) { }, } - rr, err := static.New(map[string]interface{}{"Rules": rules}) + rr, err := static.New(map[string]interface{}{"providers": providers}) if err != nil { t.Errorf("could not create registry error = %v", err) return @@ -260,11 +288,11 @@ func Test_GetAppProviders(t *testing.T) { func TestNew(t *testing.T) { tests := []struct { - name string - m map[string]interface{} - rules map[string]interface{} - want svc - wantErr interface{} + name string + m map[string]interface{} + providers map[string]interface{} + want svc + wantErr interface{} }{ { name: "no error", @@ -279,7 +307,7 @@ func TestNew(t *testing.T) { { name: "empty", m: map[string]interface{}{}, - wantErr: "error: not found: appregistrysvc: driver not found: ", + wantErr: nil, }, { name: "extra not existing field in setting", diff --git a/pkg/app/app.go b/pkg/app/app.go index d547a1e219..ec7181ca79 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -28,8 +28,11 @@ import ( // Registry is the interface that application registries implement // for discovering application providers type Registry interface { - FindProvider(ctx context.Context, mimeType string) (*registry.ProviderInfo, error) + FindProviders(ctx context.Context, mimeType string) ([]*registry.ProviderInfo, error) ListProviders(ctx context.Context) ([]*registry.ProviderInfo, error) + AddProvider(ctx context.Context, p *registry.ProviderInfo) error + GetDefaultProviderForMimeType(ctx context.Context, mimeType string) (*registry.ProviderInfo, error) + SetDefaultProviderForMimeType(ctx context.Context, mimeType string, p *registry.ProviderInfo) error } // Provider is the interface that application providers implement diff --git a/pkg/app/registry/static/static.go b/pkg/app/registry/static/static.go index bfde41d7ef..e2472ea548 100644 --- a/pkg/app/registry/static/static.go +++ b/pkg/app/registry/static/static.go @@ -35,13 +35,16 @@ func init() { } type config struct { - Rules map[string]string `mapstructure:"rules"` + Providers map[string]*registrypb.ProviderInfo `mapstructure:"providers"` } func (c *config) init() { - if len(c.Rules) == 0 { - c.Rules = map[string]string{ - "text/plain": sharedconf.GetGatewaySVC(""), + if len(c.Providers) == 0 { + c.Providers = map[string]*registrypb.ProviderInfo{ + sharedconf.GetGatewaySVC(""): ®istrypb.ProviderInfo{ + Address: sharedconf.GetGatewaySVC(""), + MimeTypes: []string{"text/plain"}, + }, } } } @@ -54,34 +57,49 @@ func parseConfig(m map[string]interface{}) (*config, error) { return c, nil } +type mimeTypeIndex struct { + defaultApp string + apps []string +} + type reg struct { - rules map[string]string + providers map[string]*registrypb.ProviderInfo + mimetypes map[string]mimeTypeIndex // map the mime type to the addresses of the corresponding providers } +// New returns an implementation of the app.Registry interface. func New(m map[string]interface{}) (app.Registry, error) { c, err := parseConfig(m) if err != nil { return nil, err } c.init() - return ®{rules: c.Rules}, nil -} -func (b *reg) ListProviders(ctx context.Context) ([]*registrypb.ProviderInfo, error) { - var providers = make([]*registrypb.ProviderInfo, 0, len(b.rules)) - for _, address := range b.rules { - providers = append(providers, ®istrypb.ProviderInfo{ - Address: address, - }) + newReg := reg{ + providers: c.Providers, + mimetypes: make(map[string]mimeTypeIndex), } - return providers, nil + + for addr, p := range c.Providers { + if p != nil { + for _, m := range p.MimeTypes { + idx, ok := newReg.mimetypes[m] + if ok { + idx.apps = append(idx.apps, addr) + } else { + newReg.mimetypes[m] = mimeTypeIndex{apps: []string{addr}} + } + } + } + } + return &newReg, nil } -func (b *reg) FindProvider(ctx context.Context, mimeType string) (*registrypb.ProviderInfo, error) { - // find the longest match +func (b *reg) FindProviders(ctx context.Context, mimeType string) ([]*registrypb.ProviderInfo, error) { + // find longest match var match string - for prefix := range b.rules { + for prefix := range b.mimetypes { if strings.HasPrefix(mimeType, prefix) && len(prefix) > len(match) { match = prefix } @@ -91,8 +109,63 @@ func (b *reg) FindProvider(ctx context.Context, mimeType string) (*registrypb.Pr return nil, errtypes.NotFound("application provider not found for mime type " + mimeType) } - p := ®istrypb.ProviderInfo{ - Address: b.rules[match], + var providers = make([]*registrypb.ProviderInfo, 0, len(b.mimetypes[match].apps)) + for _, p := range b.mimetypes[match].apps { + providers = append(providers, b.providers[p]) + } + return providers, nil +} + +func (b *reg) AddProvider(ctx context.Context, p *registrypb.ProviderInfo) error { + b.providers[p.Address] = p + + for _, m := range p.MimeTypes { + idx, ok := b.mimetypes[m] + if ok { + idx.apps = append(idx.apps, p.Address) + } else { + b.mimetypes[m] = mimeTypeIndex{apps: []string{p.Address}} + } + } + return nil +} + +func (b *reg) ListProviders(ctx context.Context) ([]*registrypb.ProviderInfo, error) { + var providers = make([]*registrypb.ProviderInfo, 0, len(b.providers)) + for _, p := range b.providers { + providers = append(providers, p) + } + return providers, nil +} + +func (b *reg) SetDefaultProviderForMimeType(ctx context.Context, mimeType string, p *registrypb.ProviderInfo) error { + idx, ok := b.mimetypes[mimeType] + if ok { + idx.defaultApp = p.Address + // Add to list of apps if not present + var present bool + for _, pr := range idx.apps { + if pr == p.Address { + present = true + break + } + } + if !present { + idx.apps = append(idx.apps, p.Address) + } + } else { + b.mimetypes[mimeType] = mimeTypeIndex{apps: []string{p.Address}, defaultApp: p.Address} + } + return nil +} + +func (b *reg) GetDefaultProviderForMimeType(ctx context.Context, mimeType string) (*registrypb.ProviderInfo, error) { + idx, ok := b.mimetypes[mimeType] + if ok { + p, ok := b.providers[idx.defaultApp] + if ok { + return p, nil + } } - return p, nil + return nil, errtypes.NotFound("default application provider not set for mime type " + mimeType) }