bug fixes: in memory allCharts map was not updated on chart refresh (#452)

* bug fix: in memory allCharts map was not updated on chart refresh

* test for single chart refresh that covers case when a new version is available

* missing delete function in charts mock

* test mock

Author: samifruit514

src/api/data/cache/cache.go

@@ -129,6 +129,38 @@ func (c *cachedCharts) Search(params charts.SearchChartsParams) ([]*swaggermodel
 	return ret, nil
 }
 
+// deleteChart is the interface implementation for data.Charts
+// It deletes Chart from memory
+// NOTE: This method does not delete from filesystem for now. It is used to test single chart refresh
+func (c *cachedCharts) DeleteChart(repoName string, chartName string, chartVersion string) error {
+
+	db, closer := c.dbSession.DB()
+	defer closer()
+
+	repo, err := models.GetRepo(db, repoName)
+	if err != nil {
+		return err
+	}
+
+	c.rwm.Lock()
+
+	for k, chart := range c.allCharts[repoName] {
+
+		if *chart.Name == chartName && *chart.Version == chartVersion {
+			log.WithFields(log.Fields{
+				"path": charthelper.DataDirBase(),
+			}).Info(chartName + " - " + chartVersion + " deleted")
+
+			c.allCharts[repo.Name] = append(c.allCharts[repo.Name][:k], c.allCharts[repo.Name][k+1:]...)
+			break
+		}
+	}
+
+	c.rwm.Unlock()
+
+	return nil
+}
+
 // Refresh is the interface implementation for data.Charts
 // It refreshes cached data for a specific repo and chart
 func (c *cachedCharts) RefreshChart(repoName string, chartName string) error {
@@ -150,22 +182,29 @@ func (c *cachedCharts) RefreshChart(repoName string, chartName string) error {
 	}
 
 	didUpdate := false
-	for _, chart := range charts {
-		if *chart.Name == chartName {
+	var alreadyExists bool
+	for _, chartFromIndex := range charts {
+		if *chartFromIndex.Name == chartName {
 			didUpdate = true
 			ch := make(chan chanItem, len(charts))
 			defer close(ch)
-			go processChartMetadata(chart, repo.URL, ch)
+			go processChartMetadata(chartFromIndex, repo.URL, ch)
 
 			it := <-ch
 			if it.err == nil {
 				c.rwm.Lock()
 				// find the key
+				alreadyExists = false
 				for k, chart := range c.allCharts[repo.Name] {
-					if chart.Name == it.chart.Name && chart.Version == it.chart.Version {
+					if *chart.Name == *it.chart.Name && *chart.Version == *it.chart.Version {
 						c.allCharts[repo.Name][k] = it.chart
+						alreadyExists = true
+						break
 					}
 				}
+				if alreadyExists == false {
+					c.allCharts[repo.Name] = append(c.allCharts[repo.Name], it.chart)
+				}
 				c.rwm.Unlock()
 			} else {
 				return it.err

src/api/data/cache/cache_test.go

@@ -107,6 +107,29 @@ func TestCachedChartsRefreshChart(t *testing.T) {
 	assert.NoErr(t, err)
 }
 
+func TestCachedChartsRefreshChartAddsNewVersion(t *testing.T) {
+	// Stubs Download and processing
+	DownloadAndExtractChartTarballOrig := charthelper.DownloadAndExtractChartTarball
+	defer func() { charthelper.DownloadAndExtractChartTarball = DownloadAndExtractChartTarballOrig }()
+	charthelper.DownloadAndExtractChartTarball = func(chart *swaggermodels.ChartPackage, repoURL string) error { return nil }
+
+	DownloadAndProcessChartIconOrig := charthelper.DownloadAndProcessChartIcon
+	defer func() { charthelper.DownloadAndProcessChartIcon = DownloadAndProcessChartIconOrig }()
+	charthelper.DownloadAndProcessChartIcon = func(chart *swaggermodels.ChartPackage) error { return nil }
+
+	chart, err := chartsImplementation.ChartVersionFromRepo("stable", "datadog", "0.1.0")
+	assert.NoErr(t, err)
+
+	err = chartsImplementation.DeleteChart("stable", *chart.Name, *chart.Version)
+	assert.NoErr(t, err)
+
+	_, err = chartsImplementation.ChartVersionFromRepo("stable", *chart.Name, *chart.Version)
+	assert.Err(t, err, errors.New("didn't find version "+*chart.Version+" of chart "+*chart.Name+"\n"))
+
+	err = chartsImplementation.RefreshChart("stable", *chart.Name)
+	assert.NoErr(t, err)
+}
+
 func TestCachedChartsRefreshChartRepoNotFound(t *testing.T) {
 	// Stubs Download and processing
 	DownloadAndExtractChartTarballOrig := charthelper.DownloadAndExtractChartTarball

src/api/data/charts.go

@@ -21,5 +21,6 @@ type Charts interface {
 	Search(params charts.SearchChartsParams) ([]*models.ChartPackage, error)
 	// Refresh freshens charts data
 	Refresh() error
-	RefreshChart(Repo string, ChartName string) error
+	RefreshChart(Repo string, chartName string) error
+	DeleteChart(Repo string, chartName string, version string) error
 }

src/api/mocks/charts.go

@@ -16,6 +16,7 @@ type MockedMethods struct {
 	All          func() ([]*models.ChartPackage, error)
 	Search       func(params chartsapi.SearchChartsParams) ([]*models.ChartPackage, error)
 	RefreshChart func(repoName string, chartName string) error
+	DeleteChart  func(repoName string, chartName string, version string) error
 }
 
 // mockCharts fulfills the data.Charts interface
@@ -146,13 +147,21 @@ func (c *mockCharts) Search(params chartsapi.SearchChartsParams) ([]*models.Char
 func (c *mockCharts) Refresh() error {
 	return nil
 }
+
 func (c *mockCharts) RefreshChart(repoName string, chartName string) error {
 	if c.mockedMethods.RefreshChart != nil {
 		return c.mockedMethods.RefreshChart(repoName, chartName)
 	}
 	return nil
 }
 
+func (c *mockCharts) DeleteChart(repoName string, chartName string, version string) error {
+	if c.mockedMethods.DeleteChart != nil {
+		return c.mockedMethods.DeleteChart(repoName, chartName, version)
+	}
+	return nil
+}
+
 // getMockRepo is a convenience that loads a yaml repo from the filesystem
 func getMockRepo(repo string) ([]byte, error) {
 	path, _ := getTestDataWd()

src/api/mocks/charts_test.go

@@ -114,3 +114,8 @@ func TestMockChartsRefreshChart(t *testing.T) {
 	err := chartsImplementation.RefreshChart(testutil.RepoName, testutil.ChartName)
 	assert.NoErr(t, err)
 }
+
+func TestMockChartsDeleteChart(t *testing.T) {
+	err := chartsImplementation.DeleteChart(testutil.RepoName, testutil.ChartName, testutil.ChartVersionString)
+	assert.NoErr(t, err)
+}