Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

Commit

Permalink
[chartsvc] Allow to return duplicated charts (#628)
Browse files Browse the repository at this point in the history
  • Loading branch information
Andres Martinez Gotor authored May 7, 2019
1 parent a929343 commit fb4e3d5
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 16 deletions.
48 changes: 32 additions & 16 deletions cmd/chartsvc/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ func getPageNumberAndSize(req *http.Request) (int, int) {
return int(pageInt), int(sizeInt)
}

// showDuplicates returns if a request wants to retrieve charts. Default false
func showDuplicates(req *http.Request) bool {
return len(req.FormValue("showDuplicates")) > 0
}

// min returns the minimum of two integers.
// We are not using math.Min since that compares float64
// and it's unnecessarily complex.
Expand All @@ -110,7 +115,7 @@ func uniqChartList(charts []*models.Chart) []*models.Chart {
return res
}

func getPaginatedChartList(repo string, pageNumber, pageSize int) (apiListResponse, interface{}, error) {
func getPaginatedChartList(repo string, pageNumber, pageSize int, showDuplicates bool) (apiListResponse, interface{}, error) {
db, closer := dbSession.DB()
defer closer()
var charts []*models.Chart
Expand All @@ -121,17 +126,20 @@ func getPaginatedChartList(repo string, pageNumber, pageSize int) (apiListRespon
pipeline = append(pipeline, bson.M{"$match": bson.M{"repo.name": repo}})
}

// We should query unique charts
pipeline = append(pipeline,
// Add a new field to store the latest version
bson.M{"$addFields": bson.M{"firstChartVersion": bson.M{"$arrayElemAt": []interface{}{"$chartversions", 0}}}},
// Group by unique digest for the latest version (remove duplicates)
bson.M{"$group": bson.M{"_id": "$firstChartVersion.digest", "chart": bson.M{"$first": "$$ROOT"}}},
// Restore original object struct
bson.M{"$replaceRoot": bson.M{"newRoot": "$chart"}},
// Order by name
bson.M{"$sort": bson.M{"name": 1}},
)
if !showDuplicates {
// We should query unique charts
pipeline = append(pipeline,
// Add a new field to store the latest version
bson.M{"$addFields": bson.M{"firstChartVersion": bson.M{"$arrayElemAt": []interface{}{"$chartversions", 0}}}},
// Group by unique digest for the latest version (remove duplicates)
bson.M{"$group": bson.M{"_id": "$firstChartVersion.digest", "chart": bson.M{"$first": "$$ROOT"}}},
// Restore original object struct
bson.M{"$replaceRoot": bson.M{"newRoot": "$chart"}},
)
}

// Order by name
pipeline = append(pipeline, bson.M{"$sort": bson.M{"name": 1}})

totalPages := 1
if pageSize != 0 {
Expand Down Expand Up @@ -166,7 +174,7 @@ func getPaginatedChartList(repo string, pageNumber, pageSize int) (apiListRespon
// listCharts returns a list of charts
func listCharts(w http.ResponseWriter, req *http.Request) {
pageNumber, pageSize := getPageNumberAndSize(req)
cl, meta, err := getPaginatedChartList("", pageNumber, pageSize)
cl, meta, err := getPaginatedChartList("", pageNumber, pageSize, showDuplicates(req))
if err != nil {
log.WithError(err).Error("could not fetch charts")
response.NewErrorResponse(http.StatusInternalServerError, "could not fetch all charts").Write(w)
Expand All @@ -178,7 +186,7 @@ func listCharts(w http.ResponseWriter, req *http.Request) {
// listRepoCharts returns a list of charts in the given repo
func listRepoCharts(w http.ResponseWriter, req *http.Request, params Params) {
pageNumber, pageSize := getPageNumberAndSize(req)
cl, meta, err := getPaginatedChartList(params["repo"], pageNumber, pageSize)
cl, meta, err := getPaginatedChartList(params["repo"], pageNumber, pageSize, showDuplicates(req))
if err != nil {
log.WithError(err).Error("could not fetch charts")
response.NewErrorResponse(http.StatusInternalServerError, "could not fetch all charts").Write(w)
Expand Down Expand Up @@ -317,7 +325,11 @@ func listChartsWithFilters(w http.ResponseWriter, req *http.Request, params Para
// continue to return empty list
}

cl := newChartListResponse(uniqChartList(charts))
chartResponse := charts
if !showDuplicates(req) {
chartResponse = uniqChartList(charts)
}
cl := newChartListResponse(chartResponse)
response.NewDataResponse(cl).Write(w)
}

Expand Down Expand Up @@ -355,7 +367,11 @@ func searchCharts(w http.ResponseWriter, req *http.Request, params Params) {
// continue to return empty list
}

cl := newChartListResponse(uniqChartList(charts))
chartResponse := charts
if !showDuplicates(req) {
chartResponse = uniqChartList(charts)
}
cl := newChartListResponse(chartResponse)
response.NewDataResponse(cl).Write(w)
}

Expand Down
33 changes: 33 additions & 0 deletions cmd/chartsvc/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -800,4 +800,37 @@ func Test_findLatestChart(t *testing.T) {
t.Errorf("Expecting %v, received %v", charts[0], data[0].ID)
}
})
t.Run("includes duplicated charts when showDuplicates param set", func(t *testing.T) {
charts := []*models.Chart{
{Name: "foo", ID: "stable/foo", Repo: models.Repo{Name: "bar"}, ChartVersions: []models.ChartVersion{models.ChartVersion{Version: "1.0.0", AppVersion: "0.1.0", Digest: "123"}}},
{Name: "foo", ID: "bitnami/foo", Repo: models.Repo{Name: "bar"}, ChartVersions: []models.ChartVersion{models.ChartVersion{Version: "1.0.0", AppVersion: "0.1.0", Digest: "123"}}},
}
reqVersion := "1.0.0"
reqAppVersion := "0.1.0"

var m mock.Mock
dbSession = mockstore.NewMockSession(&m)
m.On("All", &chartsList).Run(func(args mock.Arguments) {
*args.Get(0).(*[]*models.Chart) = charts
})

w := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/charts?showDuplicates=true&name="+charts[0].Name+"&version="+reqVersion+"&appversion="+reqAppVersion, nil)
params := Params{
"name": charts[0].Name,
"version": reqVersion,
"appversion": reqAppVersion,
}

listChartsWithFilters(w, req, params)

var b bodyAPIListResponse
json.NewDecoder(w.Body).Decode(&b)
if b.Data == nil {
t.Fatal("chart list shouldn't be null")
}
data := *b.Data

assert.Equal(t, len(data), 2, "it should return both charts")
})
}
4 changes: 4 additions & 0 deletions cmd/chartsvc/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,14 @@ func setupRoutes() http.Handler {
// Routes
apiv1 := r.PathPrefix(pathPrefix).Subrouter()
apiv1.Methods("GET").Path("/charts").Queries("name", "{chartName}", "version", "{version}", "appversion", "{appversion}").Handler(WithParams(listChartsWithFilters))
apiv1.Methods("GET").Path("/charts").Queries("name", "{chartName}", "version", "{version}", "appversion", "{appversion}", "showDuplicates", "{showDuplicates}").Handler(WithParams(listChartsWithFilters))
apiv1.Methods("GET").Path("/charts").HandlerFunc(listCharts)
apiv1.Methods("GET").Path("/charts").Queries("showDuplicates", "{showDuplicates}").HandlerFunc(listCharts)
apiv1.Methods("GET").Path("/charts/search").Queries("q", "{query}").Handler(WithParams(searchCharts))
apiv1.Methods("GET").Path("/charts/search").Queries("q", "{query}", "showDuplicates", "{showDuplicates}").Handler(WithParams(searchCharts))
apiv1.Methods("GET").Path("/charts/{repo}").Handler(WithParams(listRepoCharts))
apiv1.Methods("GET").Path("/charts/{repo}/search").Queries("q", "{query}").Handler(WithParams(searchCharts))
apiv1.Methods("GET").Path("/charts/{repo}/search").Queries("q", "{query}", "showDuplicates", "{showDuplicates}").Handler(WithParams(searchCharts))
apiv1.Methods("GET").Path("/charts/{repo}/{chartName}").Handler(WithParams(getChart))
apiv1.Methods("GET").Path("/charts/{repo}/{chartName}/versions").Handler(WithParams(listChartVersions))
apiv1.Methods("GET").Path("/charts/{repo}/{chartName}/versions/{version}").Handler(WithParams(getChartVersion))
Expand Down

0 comments on commit fb4e3d5

Please sign in to comment.