From 1bdafe9c060c42f2c84102678c6a0f3a8d3822e9 Mon Sep 17 00:00:00 2001 From: Tanvir Ahmed Date: Mon, 12 Oct 2026 11:08:58 +0000 Subject: [PATCH] refactor: use generics for plugin factory registration Replace the duplicated ConfigParser/Constructor closures for each plugin with a single generic newPluginFactory helper. The registry is now one line per plugin and the redundant *plugintypes imports are gone. Also unexport PluginFactory (only used within the package) and drop the redundant inner error wrap in the config parser. --- internal/bootstrap/plugin_factory.go | 280 +++------------------- internal/bootstrap/plugin_factory_test.go | 38 +++ 2 files changed, 72 insertions(+), 246 deletions(-) diff --git a/internal/bootstrap/plugin_factory.go b/internal/bootstrap/plugin_factory.go index e132b116..8d5c4dab 100644 --- a/internal/bootstrap/plugin_factory.go +++ b/internal/bootstrap/plugin_factory.go @@ -5,284 +5,72 @@ import ( "github.com/Authula/authula/models" accesscontrolplugin "github.com/Authula/authula/plugins/access-control" - accesscontrolplugintypes "github.com/Authula/authula/plugins/access-control/types" adminplugin "github.com/Authula/authula/plugins/admin" - adminplugintypes "github.com/Authula/authula/plugins/admin/types" apikeyplugin "github.com/Authula/authula/plugins/api-key" - apikeyplugintypes "github.com/Authula/authula/plugins/api-key/types" bearerplugin "github.com/Authula/authula/plugins/bearer" csrfplugin "github.com/Authula/authula/plugins/csrf" emailplugin "github.com/Authula/authula/plugins/email" emailpasswordplugin "github.com/Authula/authula/plugins/email-password" - emailpasswordplugintypes "github.com/Authula/authula/plugins/email-password/types" - emailplugintypes "github.com/Authula/authula/plugins/email/types" jwtplugin "github.com/Authula/authula/plugins/jwt" - jwtplugintypes "github.com/Authula/authula/plugins/jwt/types" magiclinkplugin "github.com/Authula/authula/plugins/magic-link" - magiclinkplugintypes "github.com/Authula/authula/plugins/magic-link/types" oauth2plugin "github.com/Authula/authula/plugins/oauth2" - oauth2plugintypes "github.com/Authula/authula/plugins/oauth2/types" organizationsplugin "github.com/Authula/authula/plugins/organizations" - organizationsplugintypes "github.com/Authula/authula/plugins/organizations/types" ratelimitplugin "github.com/Authula/authula/plugins/rate-limit" - ratelimitplugintypes "github.com/Authula/authula/plugins/rate-limit/types" secondarystorageplugin "github.com/Authula/authula/plugins/secondary-storage" sessionplugin "github.com/Authula/authula/plugins/session" totpplugin "github.com/Authula/authula/plugins/totp" - totplugintypes "github.com/Authula/authula/plugins/totp/types" "github.com/Authula/authula/util" ) -// PluginFactory defines a factory function for creating a plugin instance from typed config data. -type PluginFactory struct { +// pluginFactory defines how to parse a plugin's raw config into its typed config +// and how to construct the plugin from that typed config. +type pluginFactory struct { ID string ConfigParser func(rawConfig any) (any, error) // Parses raw config to typed config Constructor func(typedConfig any) models.Plugin // Creates plugin from typed config RequiredByDefault bool // Whether this plugin must be enabled } -// pluginFactories is an ordered list of registered plugin factories. -var pluginFactories = []PluginFactory{ - { - ID: models.PluginSecondaryStorage.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := secondarystorageplugin.SecondaryStoragePluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse secondary_storage plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return secondarystorageplugin.New(typedConfig.(secondarystorageplugin.SecondaryStoragePluginConfig)) - }, - }, - { - ID: models.PluginAccessControl.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := accesscontrolplugintypes.AccessControlPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse access control plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return accesscontrolplugin.New(typedConfig.(accesscontrolplugintypes.AccessControlPluginConfig)) - }, - }, - { - ID: models.PluginAdmin.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := adminplugintypes.AdminPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse admin plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return adminplugin.New(typedConfig.(adminplugintypes.AdminPluginConfig)) - }, - }, - { - ID: models.PluginEmail.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := emailplugintypes.EmailPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse email plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return emailplugin.New(typedConfig.(emailplugintypes.EmailPluginConfig)) - }, - }, - { - ID: models.PluginCSRF.String(), - RequiredByDefault: false, +// newPluginFactory builds a pluginFactory for a plugin whose config type is T and +// whose constructor returns the concrete plugin type P. Go infers T and P from the +// constructor that is passed in, so no type assertions or config parsing are repeated +// at the registration site. +func newPluginFactory[T any, P models.Plugin](id string, requiredByDefault bool, constructor func(T) P) pluginFactory { + return pluginFactory{ + ID: id, + RequiredByDefault: requiredByDefault, ConfigParser: func(rawConfig any) (any, error) { - config := csrfplugin.CSRFPluginConfig{} + var config T if rawConfig != nil { if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse csrf plugin config: %w", err) + return nil, err } } return config, nil }, Constructor: func(typedConfig any) models.Plugin { - return csrfplugin.New(typedConfig.(csrfplugin.CSRFPluginConfig)) + return constructor(typedConfig.(T)) }, - }, - { - ID: models.PluginEmailPassword.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := emailpasswordplugintypes.EmailPasswordPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse email_password plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return emailpasswordplugin.New(typedConfig.(emailpasswordplugintypes.EmailPasswordPluginConfig)) - }, - }, - { - ID: models.PluginOAuth2.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := oauth2plugintypes.OAuth2PluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse oauth2 plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return oauth2plugin.New(typedConfig.(oauth2plugintypes.OAuth2PluginConfig)) - }, - }, - { - ID: models.PluginSession.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := sessionplugin.SessionPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse session plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return sessionplugin.New(typedConfig.(sessionplugin.SessionPluginConfig)) - }, - }, - { - ID: models.PluginJWT.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := jwtplugintypes.JWTPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse jwt plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return jwtplugin.New(typedConfig.(jwtplugintypes.JWTPluginConfig)) - }, - }, - { - ID: models.PluginBearer.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := bearerplugin.BearerPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse bearer plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return bearerplugin.New(typedConfig.(bearerplugin.BearerPluginConfig)) - }, - }, - { - ID: models.PluginRateLimit.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := ratelimitplugintypes.RateLimitPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse ratelimit plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return ratelimitplugin.New(typedConfig.(ratelimitplugintypes.RateLimitPluginConfig)) - }, - }, - { - ID: models.PluginMagicLink.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := magiclinkplugintypes.MagicLinkPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse magic link plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return magiclinkplugin.New(typedConfig.(magiclinkplugintypes.MagicLinkPluginConfig)) - }, - }, - { - ID: models.PluginOrganizations.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := organizationsplugintypes.OrganizationsPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse organizations plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return organizationsplugin.New(typedConfig.(organizationsplugintypes.OrganizationsPluginConfig)) - }, - }, - { - ID: models.PluginTOTP.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := totplugintypes.TOTPPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse totp plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return totpplugin.New(typedConfig.(totplugintypes.TOTPPluginConfig)) - }, - }, - { - ID: models.PluginApiKey.String(), - RequiredByDefault: false, - ConfigParser: func(rawConfig any) (any, error) { - config := apikeyplugintypes.ApiKeyPluginConfig{} - if rawConfig != nil { - if err := util.ParsePluginConfig(rawConfig, &config); err != nil { - return nil, fmt.Errorf("failed to parse api_key plugin config: %w", err) - } - } - return config, nil - }, - Constructor: func(typedConfig any) models.Plugin { - return apikeyplugin.New(typedConfig.(apikeyplugintypes.ApiKeyPluginConfig)) - }, - }, + } +} + +// pluginFactories is an ordered list of registered plugin factories. +var pluginFactories = []pluginFactory{ + newPluginFactory(models.PluginSecondaryStorage.String(), false, secondarystorageplugin.New), + newPluginFactory(models.PluginAccessControl.String(), false, accesscontrolplugin.New), + newPluginFactory(models.PluginAdmin.String(), false, adminplugin.New), + newPluginFactory(models.PluginEmail.String(), false, emailplugin.New), + newPluginFactory(models.PluginCSRF.String(), false, csrfplugin.New), + newPluginFactory(models.PluginEmailPassword.String(), false, emailpasswordplugin.New), + newPluginFactory(models.PluginOAuth2.String(), false, oauth2plugin.New), + newPluginFactory(models.PluginSession.String(), false, sessionplugin.New), + newPluginFactory(models.PluginJWT.String(), false, jwtplugin.New), + newPluginFactory(models.PluginBearer.String(), false, bearerplugin.New), + newPluginFactory(models.PluginRateLimit.String(), false, ratelimitplugin.New), + newPluginFactory(models.PluginMagicLink.String(), false, magiclinkplugin.New), + newPluginFactory(models.PluginOrganizations.String(), false, organizationsplugin.New), + newPluginFactory(models.PluginTOTP.String(), false, totpplugin.New), + newPluginFactory(models.PluginApiKey.String(), false, apikeyplugin.New), } // isPluginEnabled checks if a plugin is enabled in the raw config diff --git a/internal/bootstrap/plugin_factory_test.go b/internal/bootstrap/plugin_factory_test.go index 73e9a0d7..8e8cd6c7 100644 --- a/internal/bootstrap/plugin_factory_test.go +++ b/internal/bootstrap/plugin_factory_test.go @@ -113,3 +113,41 @@ func TestBuildPluginsFromConfig(t *testing.T) { }) } } + +func TestPluginFactoriesAreWellFormed(t *testing.T) { + if len(pluginFactories) == 0 { + t.Fatal("expected at least one registered plugin factory") + } + + seen := make(map[string]bool, len(pluginFactories)) + + for _, factory := range pluginFactories { + t.Run(factory.ID, func(t *testing.T) { + if seen[factory.ID] { + t.Fatalf("duplicate plugin factory ID %q", factory.ID) + } + seen[factory.ID] = true + + if factory.ConfigParser == nil { + t.Fatal("ConfigParser is nil") + } + if factory.Constructor == nil { + t.Fatal("Constructor is nil") + } + + typedConfig, err := factory.ConfigParser(nil) + if err != nil { + t.Fatalf("parsing nil config: %v", err) + } + + plugin := factory.Constructor(typedConfig) + if plugin == nil { + t.Fatal("Constructor returned a nil plugin") + } + + if got := plugin.Metadata().ID; got != factory.ID { + t.Fatalf("plugin metadata ID mismatch: got %q, want %q", got, factory.ID) + } + }) + } +}