From 32e3c83b537dc735b99e9ff243c3d9f3463d7769 Mon Sep 17 00:00:00 2001 From: Karl Kirch Date: Wed, 16 Feb 2022 13:57:00 -0600 Subject: [PATCH] Feedback from 1.0 branch suggestions --- helper_test.go | 16 +++---- registry_provider.go | 28 ++++++------ registry_provider_integration_test.go | 66 +++++++++++++-------------- registry_provider_version.go | 4 +- 4 files changed, 53 insertions(+), 61 deletions(-) diff --git a/helper_test.go b/helper_test.go index 43672c49b..ffe710d76 100644 --- a/helper_test.go +++ b/helper_test.go @@ -712,12 +712,10 @@ func createPrivateRegistryProvider(t *testing.T, client *Client, org *Organizati ctx := context.Background() - privateName := PrivateRegistry - options := RegistryProviderCreateOptions{ - Name: String("tst-name-" + randomString(t)), - Namespace: &org.Name, - RegistryName: &privateName, + Name: "tst-name-" + randomString(t), + Namespace: org.Name, + RegistryName: PrivateRegistry, } prv, err := client.RegistryProviders.Create(ctx, org.Name, options) if err != nil { @@ -752,12 +750,10 @@ func createPublicRegistryProvider(t *testing.T, client *Client, org *Organizatio ctx := context.Background() - publicName := PublicRegistry - options := RegistryProviderCreateOptions{ - Name: String("tst-name-" + randomString(t)), - Namespace: String("tst-namespace-" + randomString(t)), - RegistryName: &publicName, + Name: "tst-name-" + randomString(t), + Namespace: "tst-namespace-" + randomString(t), + RegistryName: PublicRegistry, } prv, err := client.RegistryProviders.Create(ctx, org.Name, options) if err != nil { diff --git a/registry_provider.go b/registry_provider.go index 286c4a840..531596ece 100644 --- a/registry_provider.go +++ b/registry_provider.go @@ -53,8 +53,8 @@ type RegistryProvider struct { UpdatedAt string `jsonapi:"attr,updated-at"` // Relations - Organization *Organization `jsonapi:"relation,organization"` - RegistryProviderVersions []RegistryProviderVersion `jsonapi:"relation,registry-provider-version"` + Organization *Organization `jsonapi:"relation,organization"` + RegistryProviderVersions []*RegistryProviderVersion `jsonapi:"relation,registry-provider-version"` } type RegistryProviderPermissions struct { @@ -64,11 +64,11 @@ type RegistryProviderPermissions struct { type RegistryProviderListOptions struct { ListOptions // A query string to filter by registry_name - RegistryName *RegistryName `url:"filter[registry_name],omitempty"` + RegistryName RegistryName `url:"filter[registry_name],omitempty"` // A query string to filter by organization - OrganizationName *string `url:"filter[organization_name],omitempty"` + OrganizationName string `url:"filter[organization_name],omitempty"` // A query string to do a fuzzy search - Search *string `url:"q,omitempty"` + Search string `url:"q,omitempty"` } type RegistryProviderList struct { @@ -114,25 +114,25 @@ type RegistryProviderCreateOptions struct { // https://jsonapi.org/format/#crud-creating Type string `jsonapi:"primary,registry-providers"` - Namespace *string `jsonapi:"attr,namespace"` - Name *string `jsonapi:"attr,name"` - RegistryName *RegistryName `jsonapi:"attr,registry-name"` + Namespace string `jsonapi:"attr,namespace"` + Name string `jsonapi:"attr,name"` + RegistryName RegistryName `jsonapi:"attr,registry-name"` } func (o RegistryProviderCreateOptions) valid() error { - if !validString(o.Name) { + if !validString(&o.Name) { return ErrRequiredName } - if !validStringID(o.Name) { + if !validStringID(&o.Name) { return ErrInvalidName } - if !validString(o.Namespace) { + if !validString(&o.Namespace) { return errors.New("namespace is required") } - if !validStringID(o.Namespace) { + if !validStringID(&o.Namespace) { return errors.New("invalid value for namespace") } - if !validString((*string)(o.RegistryName)) { + if !validString((*string)(&o.RegistryName)) { return errors.New("registry-name is required") } return nil @@ -147,7 +147,7 @@ func (r *registryProviders) Create(ctx context.Context, organization string, opt } // Private providers must match their namespace and organization name // This is enforced by the API as well - if *options.RegistryName == PrivateRegistry && organization != *options.Namespace { + if options.RegistryName == PrivateRegistry && organization != options.Namespace { return nil, errors.New("namespace must match organization name for private providers") } diff --git a/registry_provider_integration_test.go b/registry_provider_integration_test.go index 8bf21b301..635475742 100644 --- a/registry_provider_integration_test.go +++ b/registry_provider_integration_test.go @@ -87,9 +87,8 @@ func TestRegistryProvidersList(t *testing.T) { }) t.Run("filters on registry name", func(t *testing.T) { - publicName := PublicRegistry returnedProviders, err := client.RegistryProviders.List(ctx, orgTest.Name, &RegistryProviderListOptions{ - RegistryName: &publicName, + RegistryName: PublicRegistry, ListOptions: ListOptions{ PageNumber: 0, PageSize: providerN, @@ -107,7 +106,7 @@ func TestRegistryProvidersList(t *testing.T) { break } } - assert.Equal(t, publicName, rp.RegistryName) + assert.Equal(t, PublicRegistry, rp.RegistryName) assert.True(t, foundProvider, "Expected to find provider %s but did not:\nexpected:\n%v\nreturned\n%v", rp.ID, providers, returnedProviders) } }) @@ -115,7 +114,7 @@ func TestRegistryProvidersList(t *testing.T) { t.Run("searches", func(t *testing.T) { expectedProvider := providers[0] returnedProviders, err := client.RegistryProviders.List(ctx, orgTest.Name, &RegistryProviderListOptions{ - Search: &expectedProvider.Name, + Search: expectedProvider.Name, ListOptions: ListOptions{ PageNumber: 0, PageSize: providerN, @@ -150,33 +149,30 @@ func TestRegistryProvidersCreate(t *testing.T) { orgTest, orgTestCleanup := createOrganization(t, client) defer orgTestCleanup() - publicName := PublicRegistry - privateName := PrivateRegistry - t.Run("with valid options", func(t *testing.T) { publicProviderOptions := RegistryProviderCreateOptions{ - Name: String("provider_name"), - Namespace: String("public_namespace"), - RegistryName: &publicName, + Name: "provider_name", + Namespace: "public_namespace", + RegistryName: PublicRegistry, } privateProviderOptions := RegistryProviderCreateOptions{ - Name: String("provider_name"), - Namespace: &orgTest.Name, - RegistryName: &privateName, + Name: "provider_name", + Namespace: orgTest.Name, + RegistryName: PrivateRegistry, } registryOptions := []RegistryProviderCreateOptions{publicProviderOptions, privateProviderOptions} for _, options := range registryOptions { - testName := fmt.Sprintf("with %s provider", *options.RegistryName) + testName := fmt.Sprintf("with %s provider", options.RegistryName) t.Run(testName, func(t *testing.T) { prv, err := client.RegistryProviders.Create(ctx, orgTest.Name, options) require.NoError(t, err) assert.NotEmpty(t, prv.ID) - assert.Equal(t, *options.Name, prv.Name) - assert.Equal(t, *options.Namespace, prv.Namespace) - assert.Equal(t, *options.RegistryName, prv.RegistryName) + assert.Equal(t, options.Name, prv.Name) + assert.Equal(t, options.Namespace, prv.Namespace) + assert.Equal(t, options.RegistryName, prv.RegistryName) t.Run("permissions are properly decoded", func(t *testing.T) { assert.True(t, prv.Permissions.CanDelete) @@ -197,8 +193,8 @@ func TestRegistryProvidersCreate(t *testing.T) { t.Run("with invalid options", func(t *testing.T) { t.Run("without a name", func(t *testing.T) { options := RegistryProviderCreateOptions{ - Namespace: String("namespace"), - RegistryName: &publicName, + Namespace: "namespace", + RegistryName: PublicRegistry, } rm, err := client.RegistryProviders.Create(ctx, orgTest.Name, options) assert.Nil(t, rm) @@ -207,9 +203,9 @@ func TestRegistryProvidersCreate(t *testing.T) { t.Run("with an invalid name", func(t *testing.T) { options := RegistryProviderCreateOptions{ - Name: String("invalid name"), - Namespace: String("namespace"), - RegistryName: &publicName, + Name: "invalid name", + Namespace: "namespace", + RegistryName: PublicRegistry, } rm, err := client.RegistryProviders.Create(ctx, orgTest.Name, options) assert.Nil(t, rm) @@ -218,8 +214,8 @@ func TestRegistryProvidersCreate(t *testing.T) { t.Run("without a namespace", func(t *testing.T) { options := RegistryProviderCreateOptions{ - Name: String("name"), - RegistryName: &publicName, + Name: "name", + RegistryName: PublicRegistry, } rm, err := client.RegistryProviders.Create(ctx, orgTest.Name, options) assert.Nil(t, rm) @@ -228,9 +224,9 @@ func TestRegistryProvidersCreate(t *testing.T) { t.Run("with an invalid namespace", func(t *testing.T) { options := RegistryProviderCreateOptions{ - Name: String("name"), - Namespace: String("invalid namespace"), - RegistryName: &publicName, + Name: "name", + Namespace: "invalid namespace", + RegistryName: PublicRegistry, } rm, err := client.RegistryProviders.Create(ctx, orgTest.Name, options) assert.Nil(t, rm) @@ -239,8 +235,8 @@ func TestRegistryProvidersCreate(t *testing.T) { t.Run("without a registry-name", func(t *testing.T) { options := RegistryProviderCreateOptions{ - Name: String("name"), - Namespace: String("namespace"), + Name: "name", + Namespace: "namespace", } rm, err := client.RegistryProviders.Create(ctx, orgTest.Name, options) assert.Nil(t, rm) @@ -250,9 +246,9 @@ func TestRegistryProvidersCreate(t *testing.T) { t.Run("without a valid organization", func(t *testing.T) { options := RegistryProviderCreateOptions{ - Name: String("name"), - Namespace: String("namespace"), - RegistryName: &publicName, + Name: "name", + Namespace: "namespace", + RegistryName: PublicRegistry, } rm, err := client.RegistryProviders.Create(ctx, badIdentifier, options) assert.Nil(t, rm) @@ -261,9 +257,9 @@ func TestRegistryProvidersCreate(t *testing.T) { t.Run("without a matching namespace organization.name for private registry", func(t *testing.T) { options := RegistryProviderCreateOptions{ - Name: String("name"), - Namespace: String("namespace"), - RegistryName: &privateName, + Name: "name", + Namespace: "namespace", + RegistryName: PrivateRegistry, } rm, err := client.RegistryProviders.Create(ctx, orgTest.Name, options) assert.Nil(t, rm) diff --git a/registry_provider_version.go b/registry_provider_version.go index 2bc042917..23dff9264 100644 --- a/registry_provider_version.go +++ b/registry_provider_version.go @@ -43,8 +43,8 @@ type RegistryProviderVersion struct { UpdatedAt string `jsonapi:"attr,updated-at"` // Relations - RegistryProvider *RegistryProvider `jsonapi:"relation,registry-provider"` - RegistryProviderPlatforms []RegistryProviderPlatform `jsonapi:"relation,registry-provider-platform"` + RegistryProvider *RegistryProvider `jsonapi:"relation,registry-provider"` + RegistryProviderPlatforms []*RegistryProviderPlatform `jsonapi:"relation,registry-provider-platform"` // Links Links map[string]interface{} `jsonapi:"links,omitempty"`