Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement ProviderFactory and ProviderClassFactory #3131

Merged
merged 2 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .mk/gen.mk
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ mock: ## generate mocks
mockgen -package mockghprovsvc -destination internal/providers/github/service/mock/service.go github.com/stacklok/minder/internal/providers/github/service GitHubProviderService
mockgen -package mockratecache -destination internal/providers/ratecache/mock/restcache.go github.com/stacklok/minder/internal/providers/ratecache RestClientCache
mockgen -package mockgh -destination internal/providers/github/mock/common.go github.com/stacklok/minder/internal/providers/github ClientService
mockgen -package mockprov -destination internal/providers/mock/store.go github.com/stacklok/minder/internal/providers ProviderStore


# Ugly hack: cobra uses tabs for code blocks in markdown in some places
Expand Down
108 changes: 108 additions & 0 deletions internal/providers/factory/factory.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright 2024 Stacklok, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Package factory contains logic for creating Provider instances
package factory

import (
"context"
"errors"
"fmt"

"github.com/google/uuid"

"github.com/stacklok/minder/internal/db"
"github.com/stacklok/minder/internal/providers"
v1 "github.com/stacklok/minder/pkg/providers/v1"
)

// ProviderFactory describes an interface for creating instances of a Provider
type ProviderFactory interface {
// BuildFromID creates the provider from the Provider's UUID
BuildFromID(ctx context.Context, providerID uuid.UUID) (v1.Provider, error)
// BuildFromNameProject creates the provider using the provider's name and
// project hierarchy.
BuildFromNameProject(ctx context.Context, name string, projectID uuid.UUID) (v1.Provider, error)
}

// ProviderClassFactory describes an interface for creating instances of a
// specific Provider class. The idea is that ProviderFactory determines the
// class of the Provider, and delegates to the appropraite ProviderClassFactory
type ProviderClassFactory interface {
// Build creates an instance of Provider based on the config in the DB
Build(ctx context.Context, config *db.Provider) (v1.Provider, error)
// GetSupportedClasses lists the types of Provider class which this factory
// can produce.
GetSupportedClasses() []db.ProviderClass
}

type providerFactory struct {
classFactories map[db.ProviderClass]ProviderClassFactory
store providers.ProviderStore
}

// NewProviderFactory creates a new instance of ProviderFactory
func NewProviderFactory(
classFactories []ProviderClassFactory,
store providers.ProviderStore,
) (ProviderFactory, error) {
classes := make(map[db.ProviderClass]ProviderClassFactory)
for _, factory := range classFactories {
supportedClasses := factory.GetSupportedClasses()
// Sanity check: make sure we don't inadvertently register the same
// class to two different factories, and that the factory has at least
// one type registered
if len(supportedClasses) == 0 {
return nil, errors.New("provider class factory has no registered classes")
}
for _, class := range supportedClasses {
_, ok := classes[class]
if ok {
return nil, fmt.Errorf("attempted to register class %s more than once", class)
}
classes[class] = factory
}
}
return &providerFactory{
classFactories: classes,
store: store,
}, nil
}

func (p *providerFactory) BuildFromID(ctx context.Context, providerID uuid.UUID) (v1.Provider, error) {
config, err := p.store.GetByID(ctx, providerID)
Copy link
Contributor Author

@dmjb dmjb Apr 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken to referring to db.Provider as the "provider configuration" which I feel is an accurate description of its intended purpose. In future, I may introduce a PR to rename the struct type (but not change the table, the rename can be done using sqlc's configuration). This avoids the confusion from having two types called "Provider" in the system which are used for very different things.

if err != nil {
return nil, fmt.Errorf("error retrieving db record: %w", err)
}

return p.buildFromDBRecord(ctx, config)
}

func (p *providerFactory) BuildFromNameProject(ctx context.Context, name string, projectID uuid.UUID) (v1.Provider, error) {
config, err := p.store.GetByName(ctx, projectID, name)
if err != nil {
return nil, fmt.Errorf("error retrieving db record: %w", err)
}

return p.buildFromDBRecord(ctx, config)
}

func (p *providerFactory) buildFromDBRecord(ctx context.Context, config *db.Provider) (v1.Provider, error) {
class := config.Class
factory, ok := p.classFactories[class]
if !ok {
return nil, fmt.Errorf("unexpected provider class: %s", class)
}
return factory.Build(ctx, config)
}
163 changes: 163 additions & 0 deletions internal/providers/factory/factory_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
// Copyright 2024 Stacklok, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package factory_test

import (
"context"
"testing"

"github.com/google/uuid"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"

"github.com/stacklok/minder/internal/db"
"github.com/stacklok/minder/internal/providers/factory"
"github.com/stacklok/minder/internal/providers/mock/fixtures"
minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
v1 "github.com/stacklok/minder/pkg/providers/v1"
)

// Test both create by name/project, and create by ID together.
// This is because the test logic is basically identical.
func TestProviderFactory(t *testing.T) {
t.Parallel()

scenarios := []struct {
Name string
Provider db.Provider
LookupType lookupType
RetrievalSucceeds bool
ExpectedError string
}{
{
Name: "BuildFromID returns error when DB lookup fails",
LookupType: byID,
RetrievalSucceeds: false,
ExpectedError: "error retrieving db record",
},
{
Name: "BuildFromNameProject returns error when DB lookup fails",
LookupType: byName,
RetrievalSucceeds: false,
ExpectedError: "error retrieving db record",
},
{
Name: "BuildFromID returns error when provider class has no associated factory",
Provider: providerWithClass(db.ProviderClassGithubApp),
LookupType: byID,
RetrievalSucceeds: true,
ExpectedError: "unexpected provider class",
},
{
Name: "BuildFromNameProject returns error when provider class has no associated factory",
Provider: providerWithClass(db.ProviderClassGithubApp),
LookupType: byName,
RetrievalSucceeds: true,
ExpectedError: "unexpected provider class",
},
{
Name: "BuildFromID calls factory and returns provider",
Provider: providerWithClass(db.ProviderClassGithub),
LookupType: byID,
RetrievalSucceeds: true,
},
{
Name: "BuildFromNameProject calls factory and returns provider",
Provider: providerWithClass(db.ProviderClassGithub),
LookupType: byName,
RetrievalSucceeds: true,
},
}

for _, scenario := range scenarios {
t.Run(scenario.Name, func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()
ctx := context.Background()

var opt func(mock fixtures.ProviderStoreMock)
if scenario.LookupType == byName && scenario.RetrievalSucceeds {
opt = fixtures.WithSuccessfulGetByName(&scenario.Provider)
} else if scenario.LookupType == byID && scenario.RetrievalSucceeds {
opt = fixtures.WithSuccessfulGetByID(&scenario.Provider)
} else if scenario.LookupType == byID && !scenario.RetrievalSucceeds {
opt = fixtures.WithFailedGetByID
} else {
opt = fixtures.WithFailedGetByName
}

store := fixtures.NewProviderStoreMock(opt)(ctrl)
provFactory, err := factory.NewProviderFactory(
[]factory.ProviderClassFactory{&mockClassFactory{}},
store,
)
require.NoError(t, err)

if scenario.LookupType == byName {
_, err = provFactory.BuildFromNameProject(ctx, scenario.Provider.Name, scenario.Provider.ProjectID)
} else {
_, err = provFactory.BuildFromID(ctx, scenario.Provider.ID)
}

if scenario.ExpectedError != "" {
require.ErrorContains(t, err, scenario.ExpectedError)
} else {
require.NoError(t, err)
}
})
}
}

var (
referenceProvider = db.Provider{
Name: "test-provider",
ID: uuid.New(),
ProjectID: uuid.New(),
}
)

func providerWithClass(class db.ProviderClass) db.Provider {
newProvider := referenceProvider
newProvider.Class = class
return newProvider
}

type lookupType int

const (
byID lookupType = iota
byName
)

// Not using the mock generator because we probably won't need to stub this
// elsewhere, and the implementation is trivial.
type mockClassFactory struct{}

func (_ *mockClassFactory) Build(_ context.Context, _ *db.Provider) (v1.Provider, error) {
return &mockProvider{}, nil
}

func (_ *mockClassFactory) GetSupportedClasses() []db.ProviderClass {
return []db.ProviderClass{db.ProviderClassGithub}
}

// TODO: we probably want to mock some of the provider traits in future using
// the mock generator.
type mockProvider struct{}

func (_ *mockProvider) CanImplement(_ minderv1.ProviderType) bool {
return false
}
Loading