Refactor ListInstalledPlugins (#558)

* Change ListInstalledPlugins to return receipts

* Rename function and remove private get receipts function

* Promote private function to public

* Add TODO comments

cmd/krew/cmd/list.go

@@ -40,16 +40,16 @@ Remarks:
   the names of the plugins installed. This output can be piped back to the
   "install" command.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			plugins, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
+			receipts, err := installation.GetInstalledPluginReceipts(paths.InstallReceiptsPath())
 			if err != nil {
 				return errors.Wrap(err, "failed to find all installed versions")
 			}
 
 			// return sorted list of plugin names when piped to other commands or file
 			if !isTerminal(os.Stdout) {
 				var names []string
-				for name := range plugins {
-					names = append(names, name)
+				for _, receipt := range receipts {
+					names = append(names, receipt.Name)
 				}
 				sort.Strings(names)
 				fmt.Fprintln(os.Stdout, strings.Join(names, "\n"))
@@ -58,8 +58,8 @@ Remarks:
 
 			// print table
 			var rows [][]string
-			for p, version := range plugins {
-				rows = append(rows, []string{p, version})
+			for _, receipt := range receipts {
+				rows = append(rows, []string{receipt.Name, receipt.Spec.Version})
 			}
 			rows = sortByFirstColumn(rows)
 			return printTable(os.Stdout, []string{"PLUGIN", "VERSION"}, rows)

cmd/krew/cmd/search.go

@@ -54,11 +54,17 @@ Examples:
 			pluginMap[p.Name] = p
 		}
 
-		installed, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
+		receipts, err := installation.GetInstalledPluginReceipts(paths.InstallReceiptsPath())
 		if err != nil {
 			return errors.Wrap(err, "failed to load installed plugins")
 		}
 
+		// TODO(chriskim06) include index name when refactoring for custom indexes
+		installed := make(map[string]string)
+		for _, receipt := range receipts {
+			installed[receipt.Name] = receipt.Spec.Version
+		}
+
 		var matchNames []string
 		if len(args) > 0 {
 			matches := fuzzy.Find(strings.Join(args, ""), names)

cmd/krew/cmd/update.go

@@ -120,11 +120,16 @@ func ensureIndexUpdated(_ *cobra.Command, _ []string) error {
 		return errors.Wrap(err, "failed to load plugin index after update")
 	}
 
-	installedPlugins, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
+	receipts, err := installation.GetInstalledPluginReceipts(paths.InstallReceiptsPath())
 	if err != nil {
 		return errors.Wrap(err, "failed to load installed plugins list after update")
 	}
+	installedPlugins := make(map[string]string)
+	for _, receipt := range receipts {
+		installedPlugins[receipt.Name] = receipt.Spec.Version
+	}
 
+	// TODO(chriskim06) consider commenting this out when refactoring for custom indexes
 	showUpdatedPlugins(os.Stderr, preUpdateIndex, posUpdateIndex, installedPlugins)
 
 	return nil

cmd/krew/cmd/upgrade.go

@@ -47,12 +47,12 @@ kubectl krew upgrade foo bar"`,
 			var pluginNames []string
 			if len(args) == 0 {
 				// Upgrade all plugins.
-				installed, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
+				installed, err := installation.GetInstalledPluginReceipts(paths.InstallReceiptsPath())
 				if err != nil {
 					return errors.Wrap(err, "failed to find all installed versions")
 				}
-				for name := range installed {
-					pluginNames = append(pluginNames, name)
+				for _, receipt := range installed {
+					pluginNames = append(pluginNames, receipt.Name)
 				}
 				ignoreUpgraded = true
 				skipErrors = true

internal/installation/util.go

@@ -25,24 +25,10 @@ import (
 	"sigs.k8s.io/krew/pkg/index"
 )
 
-// ListInstalledPlugins returns a list of all install plugins in a
-// name:version format based on the install receipts at the specified dir.
-func ListInstalledPlugins(receiptsDir string) (map[string]string, error) {
-	installed := make(map[string]string)
-	receipts, err := getInstalledPluginReceipts(receiptsDir)
-	if err != nil {
-		return nil, err
-	}
-	for _, r := range receipts {
-		installed[r.GetObjectMeta().GetName()] = r.Spec.Version
-	}
-	return installed, nil
-}
-
 // InstalledPluginsFromIndex returns a list of all install plugins from a particular index.
 func InstalledPluginsFromIndex(receiptsDir, indexName string) ([]index.Receipt, error) {
 	var out []index.Receipt
-	receipts, err := getInstalledPluginReceipts(receiptsDir)
+	receipts, err := GetInstalledPluginReceipts(receiptsDir)
 	if err != nil {
 		return nil, err
 	}
@@ -54,7 +40,8 @@ func InstalledPluginsFromIndex(receiptsDir, indexName string) ([]index.Receipt,
 	return out, nil
 }
 
-func getInstalledPluginReceipts(receiptsDir string) ([]index.Receipt, error) {
+// GetInstalledPluginReceipts returns a list of receipts.
+func GetInstalledPluginReceipts(receiptsDir string) ([]index.Receipt, error) {
 	files, err := filepath.Glob(filepath.Join(receiptsDir, "*"+constants.ManifestExtension))
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to glob receipts directory (%s) for manifests", receiptsDir)

internal/installation/util_test.go

@@ -33,31 +33,24 @@ func testdataPath(t *testing.T) string {
 	return filepath.Join(pwd, "testdata")
 }
 
-func TestListInstalledPlugins(t *testing.T) {
+func TestGetInstalledPluginReceipts(t *testing.T) {
 	tests := []struct {
 		name     string
-		plugins  []index.Receipt
-		expected map[string]string
+		receipts []index.Receipt
 	}{
 		{
 			name: "single plugin",
-			plugins: []index.Receipt{
+			receipts: []index.Receipt{
 				testutil.NewReceipt().WithPlugin(testutil.NewPlugin().WithName("test").WithVersion("v0.0.1").V()).V(),
 			},
-			expected: map[string]string{"test": "v0.0.1"},
 		},
 		{
 			name: "multiple plugins",
-			plugins: []index.Receipt{
+			receipts: []index.Receipt{
 				testutil.NewReceipt().WithPlugin(testutil.NewPlugin().WithName("plugin-a").WithVersion("v0.0.1").V()).V(),
 				testutil.NewReceipt().WithPlugin(testutil.NewPlugin().WithName("plugin-b").WithVersion("v0.1.0").V()).V(),
 				testutil.NewReceipt().WithPlugin(testutil.NewPlugin().WithName("plugin-c").WithVersion("v1.0.0").V()).V(),
 			},
-			expected: map[string]string{
-				"plugin-a": "v0.0.1",
-				"plugin-b": "v0.1.0",
-				"plugin-c": "v1.0.0",
-			},
 		},
 	}
 
@@ -66,15 +59,15 @@ func TestListInstalledPlugins(t *testing.T) {
 			tempDir, cleanup := testutil.NewTempDir(t)
 			defer cleanup()
 
-			for _, plugin := range test.plugins {
+			for _, plugin := range test.receipts {
 				tempDir.WriteYAML(plugin.Name+constants.ManifestExtension, plugin)
 			}
 
-			actual, err := ListInstalledPlugins(tempDir.Root())
+			actual, err := GetInstalledPluginReceipts(tempDir.Root())
 			if err != nil {
 				t.Fatal(err)
 			}
-			if diff := cmp.Diff(test.expected, actual); diff != "" {
+			if diff := cmp.Diff(test.receipts, actual); diff != "" {
 				t.Error(diff)
 			}
 		})