fix to prioritize newly added registry (#6289)

* fix to prioritize newly added registry

Signed-off-by: anandrkskd <[email protected]>

* update RegistryList func to reverse the list and return []Registry

Signed-off-by: anandrkskd <[email protected]>

* update the usage of RegistryList

Signed-off-by: anandrkskd <[email protected]>

* update unit test and mock

Signed-off-by: anandrkskd <[email protected]>

* fix: lint error and check for empty registry list

Signed-off-by: anandrkskd <[email protected]>

* add documentation

Signed-off-by: anandrkskd <[email protected]>

Signed-off-by: anandrkskd <[email protected]>

Author: anandrkskd

pkg/init/backend/flags.go

@@ -54,7 +54,7 @@ func (o *FlagsBackend) Validate(flags map[string]string, fs filesystem.Filesyste
 			return fmt.Errorf("registry %q not found in the list of devfile registries. Please use `odo preference <add/remove> registry` command to configure devfile registries", flags[FLAG_DEVFILE_REGISTRY])
 		}
 		registries := o.preferenceClient.RegistryList()
-		for _, r := range *registries {
+		for _, r := range registries {
 			isGithubRegistry, err := registry.IsGithubBasedRegistry(r.URL)
 			if err != nil {
 				return err

pkg/init/backend/flags_test.go

@@ -272,7 +272,7 @@ func TestFlagsBackend_Validate(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			prefClient := preference.NewMockClient(ctrl)
 			prefClient.EXPECT().RegistryNameExists(gomock.Any()).Return(tt.registryNameExists).AnyTimes()
-			prefClient.EXPECT().RegistryList().Return(&tt.registryList).AnyTimes()
+			prefClient.EXPECT().RegistryList().Return(tt.registryList).AnyTimes()
 
 			o := &FlagsBackend{
 				preferenceClient: prefClient,

pkg/init/init.go

@@ -175,7 +175,7 @@ func (o *InitClient) downloadFromRegistry(registryName string, devfile string, d
 
 	registries := o.preferenceClient.RegistryList()
 	var reg preference.Registry
-	for _, reg = range *registries {
+	for _, reg = range registries {
 		if forceRegistry && reg.Name == registryName {
 			err := o.registryClient.PullStackFromRegistry(reg.URL, devfile, dest, segment.GetRegistryOptions())
 			if err != nil {

pkg/init/init_test.go

@@ -44,7 +44,7 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 							URL:  "http://registry1",
 						},
 					}
-					client.EXPECT().RegistryList().Return(&registryList)
+					client.EXPECT().RegistryList().Return(registryList)
 					return client
 				},
 				registryClient: func(ctrl *gomock.Controller) registry.Client {
@@ -75,7 +75,7 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 							URL:  "http://registry1",
 						},
 					}
-					client.EXPECT().RegistryList().Return(&registryList)
+					client.EXPECT().RegistryList().Return(registryList)
 					return client
 				},
 				registryClient: func(ctrl *gomock.Controller) registry.Client {
@@ -106,7 +106,7 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 							URL:  "http://registry1",
 						},
 					}
-					client.EXPECT().RegistryList().Return(&registryList)
+					client.EXPECT().RegistryList().Return(registryList)
 					return client
 				},
 				registryClient: func(ctrl *gomock.Controller) registry.Client {
@@ -138,7 +138,7 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 							URL:  "http://registry1",
 						},
 					}
-					client.EXPECT().RegistryList().Return(&registryList)
+					client.EXPECT().RegistryList().Return(registryList)
 					return client
 				},
 				registryClient: func(ctrl *gomock.Controller) registry.Client {

pkg/odo/cli/preference/view.go

@@ -66,11 +66,11 @@ func (o *ViewOptions) RunForJsonOutput(ctx context.Context) (result interface{},
 
 	return api.PreferenceView{
 		Preferences: preferenceList.Items,
-		Registries:  *registryList,
+		Registries:  registryList,
 	}, nil
 }
 
-func HumanReadableOutput(preferenceList preference.PreferenceList, registryList *[]preference.Registry) {
+func HumanReadableOutput(preferenceList preference.PreferenceList, registryList []preference.Registry) {
 	preferenceT := ui.NewTable()
 	preferenceT.AppendHeader(table.Row{"PARAMETER", "VALUE"})
 	preferenceT.SortBy([]table.SortBy{{Name: "PARAMETER", Mode: table.Asc}})
@@ -84,9 +84,8 @@ func HumanReadableOutput(preferenceList preference.PreferenceList, registryList
 	registryT := ui.NewTable()
 	registryT.AppendHeader(table.Row{"NAME", "URL", "SECURE"})
 
-	// Loop backwards here to ensure the registry display order is correct (display latest newly added registry firstly)
-	for i := range *registryList {
-		registry := (*registryList)[i]
+	for i := range registryList {
+		registry := registryList[i]
 		secure := "No"
 		if registry.Secure {
 			secure = "Yes"
@@ -97,7 +96,7 @@ func HumanReadableOutput(preferenceList preference.PreferenceList, registryList
 	log.Info("Preference parameters:")
 	preferenceT.Render()
 	log.Info("\nDevfile registries:")
-	if registryList == nil || len(*registryList) == 0 {
+	if len(registryList) == 0 {
 		log.Warning("No devfile registries added to the configuration. Refer to `odo preference add registry -h` to add one")
 		return
 	}

pkg/odo/cli/preference/view_test.go

@@ -85,7 +85,7 @@ func TestView(t *testing.T) {
 		},
 	}
 	prefClient.EXPECT().NewPreferenceList().Return(preferenceList)
-	prefClient.EXPECT().RegistryList().Return(&registryList)
+	prefClient.EXPECT().RegistryList().Return(registryList)
 
 	err = opts.Run(context.Background())
 	if err != nil {

pkg/preference/implem.go

@@ -133,19 +133,6 @@ func newPreferenceInfo() (*preferenceInfo, error) {
 		return nil, err
 	}
 
-	regList := []Registry{}
-	if c.OdoSettings.RegistryList != nil {
-		regList = *c.OdoSettings.RegistryList
-	}
-
-	i := 0
-	j := len(regList) - 1
-	for i < j {
-		regList[i], regList[j] = regList[j], regList[i]
-		i++
-		j--
-	}
-
 	// TODO: This code block about logging warnings should be removed once users completely shift to odo v3.
 	// The warning will be printed more than once, and it can be annoying, but it should ensure that the user will change these values.
 	var requiresChange []string
@@ -437,12 +424,26 @@ func (c *preferenceInfo) ConsentTelemetry() *bool {
 	return c.OdoSettings.ConsentTelemetry
 }
 
-func (c *preferenceInfo) RegistryList() *[]Registry {
-	return c.OdoSettings.RegistryList
+// RegistryList returns the list of registries,
+// in reverse order compared to what is declared in the preferences file.
+//
+// Adding a new registry always adds it to the end of the list in the preferences file,
+// but RegistryList intentionally reverses the order to prioritize the most recently added registries.
+func (c *preferenceInfo) RegistryList() []Registry {
+	regList := make([]Registry, len(*c.OdoSettings.RegistryList))
+	copy(regList, *c.OdoSettings.RegistryList)
+	i := 0
+	j := len(regList) - 1
+	for i < j {
+		regList[i], regList[j] = regList[j], regList[i]
+		i++
+		j--
+	}
+	return regList
 }
 
 func (c *preferenceInfo) RegistryNameExists(name string) bool {
-	for _, registry := range *c.RegistryList() {
+	for _, registry := range *c.OdoSettings.RegistryList {
 		if registry.Name == name {
 			return true
 		}

pkg/preference/mock.go

@@ -23,7 +23,7 @@ type Client interface {
 	RegistryCacheTime() *time.Duration
 	EphemeralSourceVolume() *bool
 	ConsentTelemetry() *bool
-	RegistryList() *[]Registry
+	RegistryList() []Registry
 	RegistryNameExists(name string) bool
 
 	NewPreferenceList() PreferenceList

pkg/preference/preference.go

@@ -60,10 +60,8 @@ func (o RegistryClient) GetDevfileRegistries(registryName string) ([]api.Registr
 
 	hasName := len(registryName) != 0
 	if o.preferenceClient.RegistryList() != nil {
-		registryList := *o.preferenceClient.RegistryList()
-		// Loop backwards here to ensure the registry display order is correct (display latest newly added registry firstly)
-		for i := len(registryList) - 1; i >= 0; i-- {
-			registry := registryList[i]
+		registryList := o.preferenceClient.RegistryList()
+		for _, registry := range registryList {
 			if hasName {
 				if registryName == registry.Name {
 					reg := api.Registry{
@@ -241,7 +239,7 @@ func (o RegistryClient) retrieveDevfileDataFromRegistry(registryName string, dev
 	// 2. The devfile api library does not support saving in memory
 	// 3. We need to get the file from the registry and save it to the temporary file
 	// 4. We need to read the file from the temporary file, unmarshal it and then return the devfile data
-	for _, reg = range *registries {
+	for _, reg = range registries {
 		if reg.Name == registryName {
 			err = o.PullStackFromRegistry(reg.URL, devfileName, tmpFile, segment.GetRegistryOptions())
 			if err != nil {

pkg/registry/registry.go

@@ -48,13 +48,13 @@ OdoSettings:
 			registryName: "",
 			want: []api.Registry{
 				{
-					Name:   "DefaultDevfileRegistry",
-					URL:    "https://registry.devfile.io",
+					Name:   "CheDevfileRegistry",
+					URL:    "https://che-devfile-registry.openshift.io/",
 					Secure: false,
 				},
 				{
-					Name:   "CheDevfileRegistry",
-					URL:    "https://che-devfile-registry.openshift.io/",
+					Name:   "DefaultDevfileRegistry",
+					URL:    "https://registry.devfile.io",
 					Secure: false,
 				},
 			},
@@ -253,7 +253,7 @@ func TestListDevfileStacks(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			prefClient := preference.NewMockClient(ctrl)
-			prefClient.EXPECT().RegistryList().Return(&[]preference.Registry{
+			prefClient.EXPECT().RegistryList().Return([]preference.Registry{
 				{
 					Name: "TestRegistry",
 					URL:  server.URL,

pkg/registry/registry_test.go

@@ -12,7 +12,7 @@ import (
 func IsSecure(prefClient preference.Client, registryName string) bool {
 	isSecure := false
 	if prefClient.RegistryList() != nil {
-		for _, registry := range *prefClient.RegistryList() {
+		for _, registry := range prefClient.RegistryList() {
 			if registry.Name == registryName && registry.Secure {
 				isSecure = true
 				break