Skip to content

Commit

Permalink
explorer default sort by score (#3689)
Browse files Browse the repository at this point in the history
* do leave to score by default

* added test case for sort by score

* understanding scoring logic

* can deterministically sort by score

* do not sort by name by default

* do not index unstructured to get more realistic scores

* review before open pr

* just one item cause they have the same score as we are doing * query and we cannot deterministically rely on order
  • Loading branch information
enekofb authored Dec 6, 2023
1 parent a320242 commit c94cfc7
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 48 deletions.
2 changes: 1 addition & 1 deletion pkg/query/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (q *qs) RunQuery(ctx context.Context, query store.Query, opts store.QueryOp
if principal == nil {
return nil, fmt.Errorf("principal not found")
}
q.debug.Info("query received", "query", query, "principal", principal.ID)
q.debug.Info("query received", "filters", query.GetFilters(), "terms", query.GetTerms(), "principal", principal.ID)

roles, err := q.r.GetRoles(ctx)
if err != nil {
Expand Down
91 changes: 58 additions & 33 deletions pkg/query/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,8 @@ func TestRunQuery(t *testing.T) {
APIGroup: "example.com",
APIVersion: "v1",
},
{
Cluster: "test-cluster",
Name: "otherName",
Namespace: "namespace",
Kind: "ValidKind",
APIGroup: "example.com",
APIVersion: "v1",
},
},
want: []string{"someName", "otherName"},
want: []string{"someName"},
},
{
name: "get objects by cluster",
Expand Down Expand Up @@ -713,35 +705,68 @@ func TestQueryOrdering_Realistic(t *testing.T) {
authorizer: allowAll,
}

qy := &query{
orderBy: "name",
}
t.Run("query by name", func(t *testing.T) {

got, err := q.RunQuery(ctx, qy, qy)
g.Expect(err).NotTo(HaveOccurred())
qy := &query{
orderBy: "name",
}

expected := []string{
"flux-dashboards",
"flux-system",
"kube-prometheus-stack",
"kube-prometheus-stack",
"monitoring-config",
"podinfo",
"podinfo",
"podinfo",
"podinfo",
}
got, err := q.RunQuery(ctx, qy, qy)
g.Expect(err).NotTo(HaveOccurred())

actual := []string{}
for _, o := range got {
actual = append(actual, o.Name)
}
expected := []string{
"flux-dashboards",
"flux-system",
"flux-system",
"kube-prometheus-stack",
"kube-prometheus-stack",
"monitoring-config",
"podinfo",
"podinfo",
"podinfo",
"podinfo",
}

diff := cmp.Diff(expected, actual)
actual := []string{}
for _, o := range got {
actual = append(actual, o.Name)
}

diff := cmp.Diff(expected, actual)

if diff != "" {
t.Fatalf("unexpected result (-want +got):\n%s", diff)
}
})

t.Run("query by score if order selected", func(t *testing.T) {

qy := &query{
terms: "flux-system",
}

got, err := q.RunQuery(ctx, qy, qy)
g.Expect(err).NotTo(HaveOccurred())

expected := []string{
"flux-system",
"flux-system",
"monitoring-config",
"kube-prometheus-stack",
}

actual := []string{}
for _, o := range got {
actual = append(actual, o.Name)
}

diff := cmp.Diff(expected, actual)

if diff != "" {
t.Fatalf("unexpected result (-want +got):\n%s", diff)
}
})

if diff != "" {
t.Fatalf("unexpected result (-want +got):\n%s", diff)
}
}

type query struct {
Expand Down
20 changes: 7 additions & 13 deletions pkg/query/store/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ func addFieldMappings(index *mapping.IndexMappingImpl, fields []string) {
nameFieldMapping.Analyzer = "keyword"
objMapping.AddFieldMappingsAt("name", nameFieldMapping)

// do not index unstructured as for of the main index to get more realistic score
unstructuredMapping := bleve.NewDocumentDisabledMapping()
objMapping.AddSubDocumentMapping("unstructured", unstructuredMapping)

for _, field := range commonFields {
// This mapping allows us to do query-string queries on the field.
// For example, we can do `cluster:foo` to get all objects in the `foo` cluster.
Expand Down Expand Up @@ -184,7 +188,7 @@ func (i *bleveIndexer) Search(ctx context.Context, q Query, opts QueryOption) (i
defer recordIndexerMetrics(metrics.SearchAction, time.Now(), err)

// Match all by default.
// Conjunction queries will return results that match all of the subqueries.
// Conjunction queries will return results that match all the sub-queries.
query := bleve.NewConjunctionQuery(bleve.NewMatchAllQuery())

terms := q.GetTerms()
Expand Down Expand Up @@ -238,21 +242,11 @@ func (i *bleveIndexer) Search(ctx context.Context, q Query, opts QueryOption) (i
if opts.GetOffset() > 0 {
req.From = int(opts.GetOffset())
}
} else {
// Sort by name by default
sf := &search.SortField{
Field: "name",
Type: search.SortFieldAsString,
Desc: true,
}

orders = append(orders, sf)
}

// We order by score here so that we can get the most relevant results first.
orders = append(orders, &search.SortField{
Field: "_score",
Type: search.SortFieldAuto,
orders = append(orders, &search.SortScore{
Desc: true,
})

req.SortByCustom(orders)
Expand Down
17 changes: 17 additions & 0 deletions test/utils/data/explorer/sort_fixture.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,5 +124,22 @@
"unstructured": "{\"Object\":{\"kind\":\"HelmRelease\",\"apiVersion\":\"helm.toolkit.fluxcd.io/v2beta1\",\"metadata\":{\"name\":\"podinfo\",\"namespace\":\"prod-gitlab\",\"uid\":\"09f918b0-603e-4bb2-8d86-e5119b9bfc1e\",\"resourceVersion\":\"14869\",\"generation\":1,\"creationTimestamp\":\"2023-10-16T17:52:11Z\",\"annotations\":{\"kubectl.kubernetes.io/last-applied-configuration\":\"{\\\"apiVersion\\\":\\\"helm.toolkit.fluxcd.io/v2beta1\\\",\\\"kind\\\":\\\"HelmRelease\\\",\\\"metadata\\\":{\\\"annotations\\\":{},\\\"name\\\":\\\"podinfo\\\",\\\"namespace\\\":\\\"prod-gitlab\\\"},\\\"spec\\\":{\\\"chart\\\":{\\\"spec\\\":{\\\"chart\\\":\\\"podinfo\\\",\\\"interval\\\":\\\"1m\\\",\\\"sourceRef\\\":{\\\"kind\\\":\\\"HelmRepository\\\",\\\"name\\\":\\\"podinfo\\\"},\\\"version\\\":\\\"6.0.0\\\"}},\\\"interval\\\":\\\"1m\\\"}}\\n\"},\"finalizers\":[\"finalizers.fluxcd.io\"],\"managedFields\":[{\"manager\":\"Go-http-client\",\"operation\":\"Update\",\"apiVersion\":\"helm.toolkit.fluxcd.io/v2beta1\",\"time\":\"2023-10-16T17:52:11Z\",\"fieldsType\":\"FieldsV1\",\"fieldsV1\":{\"f:metadata\":{\"f:annotations\":{\".\":{},\"f:kubectl.kubernetes.io/last-applied-configuration\":{}}},\"f:spec\":{\".\":{},\"f:chart\":{\".\":{},\"f:spec\":{\".\":{},\"f:chart\":{},\"f:interval\":{},\"f:reconcileStrategy\":{},\"f:sourceRef\":{\".\":{},\"f:kind\":{},\"f:name\":{}},\"f:version\":{}}},\"f:interval\":{}}}},{\"manager\":\"helm-controller\",\"operation\":\"Update\",\"apiVersion\":\"helm.toolkit.fluxcd.io/v2beta1\",\"time\":\"2023-10-16T17:52:11Z\",\"fieldsType\":\"FieldsV1\",\"fieldsV1\":{\"f:metadata\":{\"f:finalizers\":{\".\":{},\"v:\\\"finalizers.fluxcd.io\\\"\":{}}}}},{\"manager\":\"helm-controller\",\"operation\":\"Update\",\"apiVersion\":\"helm.toolkit.fluxcd.io/v2beta1\",\"time\":\"2023-10-16T17:52:13Z\",\"fieldsType\":\"FieldsV1\",\"fieldsV1\":{\"f:status\":{\"f:conditions\":{},\"f:helmChart\":{},\"f:lastAppliedRevision\":{},\"f:lastAttemptedRevision\":{},\"f:lastAttemptedValuesChecksum\":{},\"f:lastReleaseRevision\":{},\"f:observedGeneration\":{}}},\"subresource\":\"status\"}]},\"spec\":{\"chart\":{\"spec\":{\"chart\":\"podinfo\",\"version\":\"6.0.0\",\"sourceRef\":{\"kind\":\"HelmRepository\",\"name\":\"podinfo\"},\"interval\":\"1m0s\",\"reconcileStrategy\":\"ChartVersion\"}},\"interval\":\"1m0s\"},\"status\":{\"observedGeneration\":1,\"conditions\":[{\"type\":\"Ready\",\"status\":\"True\",\"lastTransitionTime\":\"2023-10-16T17:52:13Z\",\"reason\":\"ReconciliationSucceeded\",\"message\":\"Release reconciliation succeeded\"},{\"type\":\"Released\",\"status\":\"True\",\"lastTransitionTime\":\"2023-10-16T17:52:13Z\",\"reason\":\"InstallSucceeded\",\"message\":\"Helm install succeeded\"}],\"lastAppliedRevision\":\"6.0.0\",\"lastAttemptedRevision\":\"6.0.0\",\"lastAttemptedValuesChecksum\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"lastReleaseRevision\":1,\"helmChart\":\"prod-gitlab/prod-gitlab-podinfo\"}}}",
"id": "management/prod-gitlab/helm.toolkit.fluxcd.io/v2beta1/HelmRelease/podinfo",
"tenant": ""
},
{
"cluster": "management",
"namespace": "flux-system",
"kind": "GitRepository",
"name": "flux-system",
"status": "Success",
"apiGroup": "source.toolkit.fluxcd.io",
"apiVersion": "v1",
"message": "stored artifact for revision 'main@sha1:40c8b9a245cdea9b02ad49b02e39739d87adbe68'",
"category": "source",
"unstructured": "{\"Object\":{\"kind\":\"GitRepository\",\"apiVersion\":\"source.toolkit.fluxcd.io/v1\",\"metadata\":{\"name\":\"flux-system\",\"namespace\":\"flux-system\",\"uid\":\"13921226-8f01-4683-8734-885d668f0cc3\",\"resourceVersion\":\"718464909\",\"generation\":46,\"creationTimestamp\":\"2022-06-22T16:38:18Z\",\"labels\":{\"kustomize.toolkit.fluxcd.io/name\":\"flux-system\",\"kustomize.toolkit.fluxcd.io/namespace\":\"flux-system\"},\"annotations\":{\"reconcile.fluxcd.io/requestedAt\":\"2023-11-14T15:08:00.441085515Z\"},\"finalizers\":[\"finalizers.fluxcd.io\"]},\"spec\":{\"url\":\"ssh://git@github.com/weaveworks/weave-gitops-clusters\",\"secretRef\":{\"name\":\"flux-system\"},\"interval\":\"1m0s\",\"timeout\":\"1m0s\",\"ref\":{\"branch\":\"main\"}},\"status\":{\"observedGeneration\":46,\"conditions\":[{\"type\":\"Ready\",\"status\":\"True\",\"observedGeneration\":46,\"lastTransitionTime\":\"2023-11-17T19:39:18Z\",\"reason\":\"Succeeded\",\"message\":\"stored artifact for revision 'main@sha1:40c8b9a245cdea9b02ad49b02e39739d87adbe68'\"},{\"type\":\"ArtifactInStorage\",\"status\":\"True\",\"observedGeneration\":46,\"lastTransitionTime\":\"2023-11-17T19:39:18Z\",\"reason\":\"Succeeded\",\"message\":\"stored artifact for revision 'main@sha1:40c8b9a245cdea9b02ad49b02e39739d87adbe68'\"}],\"artifact\":{\"path\":\"gitrepository/flux-system/flux-system/40c8b9a245cdea9b02ad49b02e39739d87adbe68.tar.gz\",\"url\":\"http://source-controller.flux-system.svc.cluster.local./gitrepository/flux-system/flux-system/40c8b9a245cdea9b02ad49b02e39739d87adbe68.tar.gz\",\"revision\":\"main@sha1:40c8b9a245cdea9b02ad49b02e39739d87adbe68\",\"digest\":\"sha256:9b8eb7ab3420722cf61c6b318206fefe3d5564b824d030c5b4c29717664bec79\",\"lastUpdateTime\":\"2023-11-17T19:39:18Z\",\"size\":250482},\"lastHandledReconcileAt\":\"2023-11-14T15:08:00.441085515Z\"}}}",
"id": "management/flux-system/source.toolkit.fluxcd.io/v1/GitRepository/flux-system",
"tenant": "",
"labels": {}
}
]


2 changes: 1 addition & 1 deletion ui/src/components/Explorer/Explorer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ function Explorer({
filters: queryState.filters,
limit: queryState.limit,
offset: queryState.offset,
orderBy: queryState.orderBy || 'name',
orderBy: queryState.orderBy,
descending: queryState.orderDescending || false,
category,
});
Expand Down

0 comments on commit c94cfc7

Please sign in to comment.