From e5eb673be6dcf22d2f438c8c89e9b7a35005467f Mon Sep 17 00:00:00 2001 From: Lars Karlitski Date: Mon, 2 Mar 2020 22:50:49 +0100 Subject: [PATCH] distro: rename awkwardly named function WithSingleDistro() doesn't follow go's naming convention for creating objects (New*). Rename it to NewRegistry() and rename the old NewRegistry() to NewDefaultRegistry(). The idea is that NewRegistry() can be used to create full Registry objects from outside the package. NewDefaultRegistry() is a convenience function that creates a Registry with all known distros. --- cmd/osbuild-composer/main.go | 2 +- cmd/osbuild-pipeline/main.go | 2 +- internal/distro/distro.go | 9 ++++++--- internal/distro/distro_test.go | 2 +- internal/distro/fedora30/distro_test.go | 4 ++-- internal/distro/fedora31/distro_test.go | 4 ++-- internal/distro/fedora32/distro_test.go | 4 ++-- internal/distro/rhel81/distro_test.go | 4 ++-- internal/distro/rhel82/distro_test.go | 4 ++-- internal/jobqueue/api_test.go | 6 +++--- internal/jobqueue/job.go | 2 +- internal/mocks/distro/distro_mock.go | 4 ++-- internal/mocks/rpmmd/fixtures.go | 4 ++-- internal/rcm/api_test.go | 4 ++-- 14 files changed, 29 insertions(+), 26 deletions(-) diff --git a/cmd/osbuild-composer/main.go b/cmd/osbuild-composer/main.go index 4aadf84f2..1b6b85ac6 100644 --- a/cmd/osbuild-composer/main.go +++ b/cmd/osbuild-composer/main.go @@ -80,7 +80,7 @@ func main() { } rpm := rpmmd.NewRPMMD(path.Join(cacheDirectory, "rpmmd")) - distros := distro.NewRegistry([]string{"/etc/osbuild-composer", "/usr/share/osbuild-composer"}) + distros := distro.NewDefaultRegistry([]string{"/etc/osbuild-composer", "/usr/share/osbuild-composer"}) distribution, err := distros.FromHost() if err != nil { diff --git a/cmd/osbuild-pipeline/main.go b/cmd/osbuild-pipeline/main.go index b95343d2d..59c69f61f 100644 --- a/cmd/osbuild-pipeline/main.go +++ b/cmd/osbuild-pipeline/main.go @@ -64,7 +64,7 @@ func main() { } } - distros := distro.NewRegistry([]string{"."}) + distros := distro.NewDefaultRegistry([]string{"."}) d := distros.GetDistro(distroArg) if d == nil { panic("unknown distro: " + distroArg) diff --git a/internal/distro/distro.go b/internal/distro/distro.go index 56432f211..751981c23 100644 --- a/internal/distro/distro.go +++ b/internal/distro/distro.go @@ -70,15 +70,18 @@ type Registry struct { distros map[common.Distribution]Distro } -func WithSingleDistro(dist Distro) *Registry { +func NewRegistry(distros ...Distro) *Registry { reg := &Registry{ distros: make(map[common.Distribution]Distro), } - reg.register(dist) + for _, dist := range distros { + reg.register(dist) + } return reg } -func NewRegistry(confPaths []string) *Registry { +// Create a new Registry containing all known distros. +func NewDefaultRegistry(confPaths []string) *Registry { distros := &Registry{ distros: make(map[common.Distribution]Distro), } diff --git a/internal/distro/distro_test.go b/internal/distro/distro_test.go index 26ae738a0..615497bfb 100644 --- a/internal/distro/distro_test.go +++ b/internal/distro/distro_test.go @@ -42,7 +42,7 @@ func TestDistro_Pipeline(t *testing.T) { continue } t.Run(tt.Compose.OutputFormat, func(t *testing.T) { - distros := distro.NewRegistry([]string{"../.."}) + distros := distro.NewDefaultRegistry([]string{"../.."}) d := distros.GetDistro(tt.Compose.Distro) if d == nil { t.Errorf("unknown distro: %v", tt.Compose.Distro) diff --git a/internal/distro/fedora30/distro_test.go b/internal/distro/fedora30/distro_test.go index 1e6e69975..32ec443a7 100644 --- a/internal/distro/fedora30/distro_test.go +++ b/internal/distro/fedora30/distro_test.go @@ -19,7 +19,7 @@ func TestListOutputFormats(t *testing.T) { "vmdk", } - distros := distro.NewRegistry([]string{"../../../"}) + distros := distro.NewDefaultRegistry([]string{"../../../"}) f30 := distros.GetDistro("fedora-30") if got := f30.ListOutputFormats(); !reflect.DeepEqual(got, want) { t.Errorf("ListOutputFormats() = %v, want %v", got, want) @@ -93,7 +93,7 @@ func TestFilenameFromType(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - distros := distro.NewRegistry([]string{"../../../"}) + distros := distro.NewDefaultRegistry([]string{"../../../"}) f30 := distros.GetDistro("fedora-30") got, got1, err := f30.FilenameFromType(tt.args.outputFormat) if (err != nil) != tt.wantErr { diff --git a/internal/distro/fedora31/distro_test.go b/internal/distro/fedora31/distro_test.go index b138f38fd..9c5faba27 100644 --- a/internal/distro/fedora31/distro_test.go +++ b/internal/distro/fedora31/distro_test.go @@ -19,7 +19,7 @@ func TestListOutputFormats(t *testing.T) { "vmdk", } - distros := distro.NewRegistry([]string{"../../../"}) + distros := distro.NewDefaultRegistry([]string{"../../../"}) f31 := distros.GetDistro("fedora-31") if got := f31.ListOutputFormats(); !reflect.DeepEqual(got, want) { t.Errorf("ListOutputFormats() = %v, want %v", got, want) @@ -93,7 +93,7 @@ func TestFilenameFromType(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - distros := distro.NewRegistry([]string{"../../../"}) + distros := distro.NewDefaultRegistry([]string{"../../../"}) f31 := distros.GetDistro("fedora-31") got, got1, err := f31.FilenameFromType(tt.args.outputFormat) if (err != nil) != tt.wantErr { diff --git a/internal/distro/fedora32/distro_test.go b/internal/distro/fedora32/distro_test.go index 0ceb192ab..4fee332ac 100644 --- a/internal/distro/fedora32/distro_test.go +++ b/internal/distro/fedora32/distro_test.go @@ -19,7 +19,7 @@ func TestListOutputFormats(t *testing.T) { "vmdk", } - distros := distro.NewRegistry([]string{"../../../"}) + distros := distro.NewDefaultRegistry([]string{"../../../"}) f32 := distros.GetDistro("fedora-32") if got := f32.ListOutputFormats(); !reflect.DeepEqual(got, want) { t.Errorf("ListOutputFormats() = %v, want %v", got, want) @@ -93,7 +93,7 @@ func TestFilenameFromType(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - distros := distro.NewRegistry([]string{"../../../"}) + distros := distro.NewDefaultRegistry([]string{"../../../"}) f32 := distros.GetDistro("fedora-32") got, got1, err := f32.FilenameFromType(tt.args.outputFormat) if (err != nil) != tt.wantErr { diff --git a/internal/distro/rhel81/distro_test.go b/internal/distro/rhel81/distro_test.go index 9950c1eb2..ea71f6746 100644 --- a/internal/distro/rhel81/distro_test.go +++ b/internal/distro/rhel81/distro_test.go @@ -19,7 +19,7 @@ func TestListOutputFormats(t *testing.T) { "vmdk", } - distros := distro.NewRegistry([]string{"../../../"}) + distros := distro.NewDefaultRegistry([]string{"../../../"}) rhel81 := distros.GetDistro("rhel-8.2") if got := rhel81.ListOutputFormats(); !reflect.DeepEqual(got, want) { t.Errorf("ListOutputFormats() = %v, want %v", got, want) @@ -93,7 +93,7 @@ func TestFilenameFromType(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - distros := distro.NewRegistry([]string{"../../../"}) + distros := distro.NewDefaultRegistry([]string{"../../../"}) rhel81 := distros.GetDistro("rhel-8.2") got, got1, err := rhel81.FilenameFromType(tt.args.outputFormat) if (err != nil) != tt.wantErr { diff --git a/internal/distro/rhel82/distro_test.go b/internal/distro/rhel82/distro_test.go index 3dea1259c..6201f2918 100644 --- a/internal/distro/rhel82/distro_test.go +++ b/internal/distro/rhel82/distro_test.go @@ -19,7 +19,7 @@ func TestListOutputFormats(t *testing.T) { "vmdk", } - distros := distro.NewRegistry([]string{"../../../"}) + distros := distro.NewDefaultRegistry([]string{"../../../"}) rhel82 := distros.GetDistro("rhel-8.2") if got := rhel82.ListOutputFormats(); !reflect.DeepEqual(got, want) { t.Errorf("ListOutputFormats() = %v, want %v", got, want) @@ -93,7 +93,7 @@ func TestFilenameFromType(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - distros := distro.NewRegistry([]string{"../../../"}) + distros := distro.NewDefaultRegistry([]string{"../../../"}) rhel82 := distros.GetDistro("rhel-8.2") got, got1, err := rhel82.FilenameFromType(tt.args.outputFormat) if (err != nil) != tt.wantErr { diff --git a/internal/jobqueue/api_test.go b/internal/jobqueue/api_test.go index 69a5e1bd1..ddc521068 100644 --- a/internal/jobqueue/api_test.go +++ b/internal/jobqueue/api_test.go @@ -34,7 +34,7 @@ func TestBasic(t *testing.T) { for _, c := range cases { distroStruct := test_distro.New() - registry := distro_mock.NewRegistry() + registry := distro_mock.NewDefaultRegistry() api := jobqueue.New(nil, store.New(nil, distroStruct, *registry)) test.TestNonJsonRoute(t, api, false, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedResponse) @@ -44,7 +44,7 @@ func TestBasic(t *testing.T) { func TestCreate(t *testing.T) { id, _ := uuid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff") distroStruct := test_distro.New() - registry := distro_mock.NewRegistry() + registry := distro_mock.NewDefaultRegistry() store := store.New(nil, distroStruct, *registry) api := jobqueue.New(nil, store) @@ -60,7 +60,7 @@ func TestCreate(t *testing.T) { func testUpdateTransition(t *testing.T, from, to string, expectedStatus int, expectedResponse string) { id, _ := uuid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff") distroStruct := test_distro.New() - registry := distro_mock.NewRegistry() + registry := distro_mock.NewDefaultRegistry() store := store.New(nil, distroStruct, *registry) api := jobqueue.New(nil, store) diff --git a/internal/jobqueue/job.go b/internal/jobqueue/job.go index 5e63ada4a..cd804fc07 100644 --- a/internal/jobqueue/job.go +++ b/internal/jobqueue/job.go @@ -46,7 +46,7 @@ func (e *TargetsError) Error() string { } func (job *Job) Run(uploader LocalTargetUploader) (*common.ComposeResult, error) { - distros := distro.NewRegistry([]string{"/etc/osbuild-composer", "/usr/share/osbuild-composer"}) + distros := distro.NewDefaultRegistry([]string{"/etc/osbuild-composer", "/usr/share/osbuild-composer"}) d := distros.GetDistro(job.Distro) if d == nil { return nil, fmt.Errorf("unknown distro: %s", job.Distro) diff --git a/internal/mocks/distro/distro_mock.go b/internal/mocks/distro/distro_mock.go index 178ee3e93..cd950792c 100644 --- a/internal/mocks/distro/distro_mock.go +++ b/internal/mocks/distro/distro_mock.go @@ -5,10 +5,10 @@ import ( "github.com/osbuild/osbuild-composer/internal/distro/fedoratest" ) -func NewRegistry() *distro.Registry { +func NewDefaultRegistry() *distro.Registry { ftest := fedoratest.New() if ftest == nil { panic("Attempt to register Fedora test failed") } - return distro.WithSingleDistro(ftest) + return distro.NewRegistry(ftest) } diff --git a/internal/mocks/rpmmd/fixtures.go b/internal/mocks/rpmmd/fixtures.go index 7b8c2f7ca..4ceb6e858 100644 --- a/internal/mocks/rpmmd/fixtures.go +++ b/internal/mocks/rpmmd/fixtures.go @@ -94,7 +94,7 @@ func createBaseStoreFixture() *store.Store { } d := test_distro.New() - r := distro_mock.NewRegistry() + r := distro_mock.NewDefaultRegistry() s := store.New(nil, d, *r) s.Blueprints[bName] = b @@ -191,7 +191,7 @@ func createStoreWithoutComposesFixture() *store.Store { } d := test_distro.New() - r := distro_mock.NewRegistry() + r := distro_mock.NewDefaultRegistry() s := store.New(nil, d, *r) s.Blueprints[bName] = b diff --git a/internal/rcm/api_test.go b/internal/rcm/api_test.go index 1e0009d00..269b10514 100644 --- a/internal/rcm/api_test.go +++ b/internal/rcm/api_test.go @@ -49,7 +49,7 @@ func TestBasicRcmAPI(t *testing.T) { {"GET", "/v1/compose/7802c476-9cd1-41b7-ba81-43c1906bce73", `{"status":"RUNNING"}`, "application/json", http.StatusBadRequest, `{"error_reason":"Compose UUID does not exist"}`}, } - registry := distro_mock.NewRegistry() + registry := distro_mock.NewDefaultRegistry() distroStruct := fedoratest.New() api := rcm.New(nil, store.New(nil, distroStruct, *registry), rpmmd_mock.NewRPMMDMock(rpmmd_mock.BaseFixture())) @@ -74,7 +74,7 @@ func TestBasicRcmAPI(t *testing.T) { func TestSubmitCompose(t *testing.T) { // Test the most basic use case: Submit a new job and get its status. distroStruct := fedoratest.New() - registry := distro_mock.NewRegistry() + registry := distro_mock.NewDefaultRegistry() api := rcm.New(nil, store.New(nil, distroStruct, *registry), rpmmd_mock.NewRPMMDMock(rpmmd_mock.BaseFixture())) var submit_reply struct {