Skip to content

Commit

Permalink
Remove RawIcon from chart versions (#1460)
Browse files Browse the repository at this point in the history
  • Loading branch information
absoludity authored and Andres Martinez Gotor committed Jan 16, 2020
1 parent 5457360 commit 158da32
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 8 deletions.
12 changes: 10 additions & 2 deletions cmd/assetsvc/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ func newChartResponse(c *models.Chart) *apiResponse {
return &apiResponse{
Type: "chart",
ID: c.ID,
Attributes: chartAttributes(*c),
Attributes: blankRawIcon(chartAttributes(*c)),
Links: selfLink{pathPrefix + "/charts/" + c.ID},
Relationships: relMap{
"latestChartVersion": rel{
Expand All @@ -314,6 +314,14 @@ func newChartResponse(c *models.Chart) *apiResponse {
}
}

// blankRawIcon returns the same chart data but with a blank raw icon field.
// TODO(mnelson): The raw icon data should be stored in a separate postgresql column
// rather than the json field so that this isn't necessary.
func blankRawIcon(c models.Chart) models.Chart {
c.RawIcon = nil
return c
}

func newChartListResponse(charts []*models.Chart) apiListResponse {
cl := apiListResponse{}
for _, c := range charts {
Expand Down Expand Up @@ -346,7 +354,7 @@ func newChartVersionResponse(c *models.Chart, cv models.ChartVersion) *apiRespon
Links: selfLink{pathPrefix + "/charts/" + c.ID + "/versions/" + cv.Version},
Relationships: relMap{
"chart": rel{
Data: chartAttributes(*c),
Data: blankRawIcon(chartAttributes(*c)),
Links: selfLink{pathPrefix + "/charts/" + c.ID},
},
},
Expand Down
33 changes: 27 additions & 6 deletions cmd/assetsvc/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ func Test_newChartResponse(t *testing.T) {
{"chart has many versions", models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.2"}, {Version: "0.1.0"}},
}},
{"raw_icon is never sent down the wire", models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "1.2.3"}}, RawIcon: iconBytes(), IconContentType: "image/svg",
}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -132,6 +135,8 @@ func Test_newChartResponse(t *testing.T) {
assert.Equal(t, cResponse.Relationships["latestChartVersion"].Data.(models.ChartVersion).Version, tt.chart.ChartVersions[0].Version, "latestChartVersion should match version at index 0")
assert.Equal(t, cResponse.Links.(selfLink).Self, pathPrefix+"/charts/"+tt.chart.ID, "self link should be the same")
assert.Equal(t, len(cResponse.Attributes.(models.Chart).ChartVersions), len(tt.chart.ChartVersions), "number of chart versions in the response should be the same")
// We don't send the raw icon down the wire.
assert.Nil(t, cResponse.Attributes.(models.Chart).RawIcon)
})
}
}
Expand Down Expand Up @@ -174,12 +179,23 @@ func Test_newChartListResponse(t *testing.T) {

func Test_newChartVersionResponse(t *testing.T) {
tests := []struct {
name string
chart models.Chart
name string
chart models.Chart
expectedIcon string
}{
{"my-chart", models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}, {Version: "0.2.3"}},
}},
{
name: "my-chart",
chart: models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}, {Version: "0.2.3"}},
},
},
{
name: "RawIcon is never sent down the wire",
chart: models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "1.2.3"}}, RawIcon: iconBytes(), IconContentType: "image/svg",
},
expectedIcon: "/v1/assets/my-repo/my-chart/logo",
},
}

for _, tt := range tests {
Expand All @@ -190,7 +206,12 @@ func Test_newChartVersionResponse(t *testing.T) {
assert.Equal(t, cvResponse.ID, tt.chart.ID+"-"+tt.chart.ChartVersions[i].Version, "reponse id should have chart version suffix")
assert.Equal(t, cvResponse.Links.(interface{}).(selfLink).Self, pathPrefix+"/charts/"+tt.chart.ID+"/versions/"+tt.chart.ChartVersions[i].Version, "self link should be the same")
assert.Equal(t, cvResponse.Attributes.(models.ChartVersion).Version, tt.chart.ChartVersions[i].Version, "chart version in the response should be the same")
assert.Equal(t, cvResponse.Relationships["chart"].Data.(interface{}).(models.Chart), tt.chart, "chart in relatioship matches")

// The chart should have had its icon url set and raw icon data removed.
expectedChart := tt.chart
expectedChart.RawIcon = nil
expectedChart.Icon = tt.expectedIcon
assert.Equal(t, cvResponse.Relationships["chart"].Data.(interface{}).(models.Chart), expectedChart, "chart in relatioship matches")
}
})
}
Expand Down

0 comments on commit 158da32

Please sign in to comment.