From d33ef8d79c1fe7d67d8fd9ddaeb39bf00911b78d Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Tue, 29 Mar 2022 11:58:29 +0300 Subject: [PATCH] app/products: add dependency resolver (#19847) * app/products: add dependency resolver * reflect review comments --- app/channels.go | 12 +++- app/product.go | 73 +++++++++++++++++++++-- app/product_test.go | 140 ++++++++++++++++++++++++++++++++++++++++++++ app/server.go | 10 +--- 4 files changed, 221 insertions(+), 14 deletions(-) create mode 100644 app/product_test.go diff --git a/app/channels.go b/app/channels.go index b8ae9a5450..da69a68dce 100644 --- a/app/channels.go +++ b/app/channels.go @@ -96,13 +96,19 @@ type Channels struct { } func init() { - RegisterProduct("channels", func(s *Server, services map[ServiceKey]interface{}) (Product, error) { - return NewChannels(s, services) + RegisterProduct("channels", ProductManifest{ + Initializer: func(s *Server, services map[ServiceKey]interface{}) (Product, error) { + return NewChannels(s, services) + }, + Dependencies: map[ServiceKey]struct{}{ + ConfigKey: {}, + LicenseKey: {}, + FilestoreKey: {}, + }, }) } func NewChannels(s *Server, services map[ServiceKey]interface{}) (*Channels, error) { - ch := &Channels{ srv: s, imageProxy: imageproxy.MakeImageProxy(s, s.httpService, s.Log), diff --git a/app/product.go b/app/product.go index e45e6faee7..2e09f137c8 100644 --- a/app/product.go +++ b/app/product.go @@ -3,13 +3,78 @@ package app +import ( + "fmt" + "strings" +) + type Product interface { Start() error Stop() error } -var products = make(map[string]func(*Server, map[ServiceKey]interface{}) (Product, error)) - -func RegisterProduct(name string, f func(*Server, map[ServiceKey]interface{}) (Product, error)) { - products[name] = f +type ProductManifest struct { + Initializer func(*Server, map[ServiceKey]interface{}) (Product, error) + Dependencies map[ServiceKey]struct{} +} + +var products = make(map[string]ProductManifest) + +func RegisterProduct(name string, m ProductManifest) { + products[name] = m +} + +func (s *Server) initializeProducts( + productMap map[string]ProductManifest, + serviceMap map[ServiceKey]interface{}, +) error { + // create a product map to consume + pmap := make(map[string]struct{}) + for name := range productMap { + pmap[name] = struct{}{} + } + + // We figure out the initialization order by trial and error fashion hence maxTry + // is the maximum possible trials of initialization attempts. The order is not + // determined elsewhere therefore we do a on the fly sorting here. Which means the + // initialization order will be resolved during the loop. + maxTry := len(pmap) * len(pmap) + + for len(pmap) > 0 && maxTry != 0 { + initLoop: + for product := range pmap { + manifest := productMap[product] + // we have dependencies defined. Here we check if the serviceMap + // has all the dependencies registered. If not, we continue to the + // loop to let other products initialize and register their services + // if they have any. + for key := range manifest.Dependencies { + if _, ok := serviceMap[key]; !ok { + maxTry-- + continue initLoop + } + } + + // some products can register themselves/their services + initializer := manifest.Initializer + prod, err := initializer(s, serviceMap) + if err != nil { + return fmt.Errorf("error initializing product %q: %w", product, err) + } + s.products[product] = prod + + // we remove this product from the map to not try to initialize it again + delete(pmap, product) + } + } + + if maxTry == 0 && len(pmap) != 0 { + var products string + for p := range pmap { + products = strings.Join([]string{products, fmt.Sprintf("%q", p)}, " ") + } + return fmt.Errorf("could not initialize product(s) due to circular dependency: %s", products) + } + + return nil } diff --git a/app/product_test.go b/app/product_test.go new file mode 100644 index 0000000000..1bfd408773 --- /dev/null +++ b/app/product_test.go @@ -0,0 +1,140 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +const ( + testSrvKey1 = "test_1" + testSrvKey2 = "test_2" +) + +type productA struct{} + +func newProductA(s *Server, m map[ServiceKey]interface{}) (Product, error) { + m[testSrvKey1] = nil + return &productA{}, nil +} + +func (p *productA) Start() error { return nil } +func (p *productA) Stop() error { return nil } + +type productB struct{} + +func newProductB(s *Server, m map[ServiceKey]interface{}) (Product, error) { + m[testSrvKey2] = nil + return &productB{}, nil +} + +func (p *productB) Start() error { return nil } +func (p *productB) Stop() error { return nil } + +func TestInitializeProducts(t *testing.T) { + t.Run("2 products and no circular dependency", func(t *testing.T) { + serviceMap := map[ServiceKey]interface{}{ + ConfigKey: nil, + LicenseKey: nil, + FilestoreKey: nil, + ClusterKey: nil, + } + + products := map[string]ProductManifest{ + "productA": { + Initializer: newProductA, + Dependencies: map[ServiceKey]struct{}{ + ConfigKey: {}, + LicenseKey: {}, + FilestoreKey: {}, + ClusterKey: {}, + }, + }, + "productB": { + Initializer: newProductB, + Dependencies: map[ServiceKey]struct{}{ + ConfigKey: {}, + testSrvKey1: {}, + FilestoreKey: {}, + ClusterKey: {}, + }, + }, + } + server := &Server{ + products: make(map[string]Product), + } + + err := server.initializeProducts(products, serviceMap) + require.NoError(t, err) + require.Len(t, server.products, 2) + }) + + t.Run("2 products and circular dependency", func(t *testing.T) { + serviceMap := map[ServiceKey]interface{}{ + ConfigKey: nil, + LicenseKey: nil, + FilestoreKey: nil, + ClusterKey: nil, + } + + products := map[string]ProductManifest{ + "productA": { + Initializer: newProductA, + Dependencies: map[ServiceKey]struct{}{ + ConfigKey: {}, + LicenseKey: {}, + FilestoreKey: {}, + ClusterKey: {}, + testSrvKey2: {}, + }, + }, + "productB": { + Initializer: newProductB, + Dependencies: map[ServiceKey]struct{}{ + ConfigKey: {}, + testSrvKey1: {}, + FilestoreKey: {}, + ClusterKey: {}, + }, + }, + } + server := &Server{ + products: make(map[string]Product), + } + + err := server.initializeProducts(products, serviceMap) + require.Error(t, err) + }) + + t.Run("2 products and one w/o any dependency", func(t *testing.T) { + serviceMap := map[ServiceKey]interface{}{ + ConfigKey: nil, + LicenseKey: nil, + FilestoreKey: nil, + ClusterKey: nil, + } + + products := map[string]ProductManifest{ + "productA": { + Initializer: newProductA, + Dependencies: map[ServiceKey]struct{}{ + ConfigKey: {}, + LicenseKey: {}, + }, + }, + "productB": { + Initializer: newProductB, + }, + } + server := &Server{ + products: make(map[string]Product), + } + + err := server.initializeProducts(products, serviceMap) + require.NoError(t, err) + require.Len(t, server.products, 2) + }) +} diff --git a/app/server.go b/app/server.go index 4f473d2e97..1e3ed63161 100644 --- a/app/server.go +++ b/app/server.go @@ -378,13 +378,9 @@ func NewServer(options ...Option) (*Server, error) { // Step 8: Initialize products. // Depends on s.httpService. - for name, initializer := range products { - prod, err2 := initializer(s, serviceMap) - if err2 != nil { - return nil, errors.Wrapf(err2, "error initializing product: %s", name) - } - - s.products[name] = prod + err = s.initializeProducts(products, serviceMap) + if err != nil { + return nil, errors.Wrap(err, "failed to initialize products") } // It is important to initialize the hub only after the global logger is set