Feedback from 1.0 branch suggestions

hashicorp · Feb 16, 2022 · 32e3c83 · 32e3c83
1 parent 7dd02ee
commit 32e3c83
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 61 deletions.
diff --git 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
@@ -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
@@ -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,15 +106,15 @@ 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)
 			}
 		})
 
 		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
@@ -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"`