Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ENHANCEMENT] Add query param 'metadata_only' on almost all query endpoints #1993

Merged
merged 3 commits into from
May 15, 2024
Merged

[ENHANCEMENT] Add query param 'metadata_only' on almost all query endpoints #1993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1aeae1d
[ENHANCEMENT] Add query param 'metadata_only' on almost all query end…
Nexucis May 14, 2024
6c04bdf
make ui get dashboard more generic
Nexucis May 15, 2024
6fa3f21
add e2e test
Nexucis May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions internal/api/dashboard/cleaner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package dashboard

import (
"context"
"github.com/perses/perses/pkg/model/api"
"testing"
"time"

Expand All @@ -32,6 +33,18 @@ func (d *mockDAO) List(_ *ephemeraldashboard.Query) ([]*v1.EphemeralDashboard, e
return d.dashboards, nil
}

func (d *mockDAO) MetadataList(_ *ephemeraldashboard.Query) ([]api.Entity, error) {
var result []api.Entity
for _, dashboard := range d.dashboards {
result = append(result, &v1.PartialProjectEntity{
Kind: dashboard.Kind,
Metadata: dashboard.Metadata,
Spec: struct{}{},
})
}
return result, nil
}

func (d *mockDAO) Delete(project string, name string) error {
for ed := range d.dashboards {
if d.dashboards[ed].Metadata.Project == project && d.dashboards[ed].Metadata.Name == name {
Expand Down
1 change: 1 addition & 0 deletions internal/api/database/model/dao.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
)

type Query interface {
GetMetadataOnlyQueryParam() bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's open the debate that we all expect: should it be metadata_only or something else?

Food for think:
What if the default was to actually give only the metadata to prevent from dumb requests. You could have full=true for example in all the queries that actually want details

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

partial, full, skip_spec, ignore_spec, ...
Depends on the the default behavior too, "full=true" will not work with the current implem

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's what I'm saying. Changing the default behaviour to by default having full=false would in a sense give a message that if you use full=true you don't use default and don't complain if it fails or take time to answer, or is paginated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you get the list of data with no query parameter, you won't expect a partial response data.
What we can do is to provide a pagination so you cannot get everything excepting if you explicitly say it.

metadata_only is at least explicitly saying you will only get the metadata and nothing else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but before including a pagination, I want to make everything to be able to get all data without crashing Perses.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, just wanted to make sure we explore all the possibilities.

I think I'd tend to be in favor of an API that by default gives everything and light than something very detailed and potentially paginated in the future. But at the end as long as the light mode is available I'm good with any of the default behaviour.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pagination for the win 👍🏽

}

type DAO interface {
Expand Down
22 changes: 22 additions & 0 deletions internal/api/e2e/api/dashboard_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,28 @@ func TestListDashboardInEmptyProject(t *testing.T) {
})
}

func TestListDashboardWithOnlyMetadata(t *testing.T) {
e2eframework.WithServer(t, func(expect *httpexpect.Expect, manager dependency.PersistenceManager) []api.Entity {
demoDashboard := e2eframework.NewDashboard(t, "perses", "Demo")
persesProject := e2eframework.NewProject("perses")
e2eframework.CreateAndWaitUntilEntitiesExist(t, manager, persesProject, demoDashboard)

response := expect.GET(fmt.Sprintf("%s/%s/%s/%s", utils.APIV1Prefix, utils.PathProject, persesProject.GetMetadata().GetName(), utils.PathDashboard)).
WithQuery("metadata_only", true).
Expect().
Status(http.StatusOK)

response.JSON().Array().Length().IsEqual(1)
response.JSON().Array().Value(0).Object().IsEqual(modelV1.PartialProjectEntity{
Kind: demoDashboard.Kind,
Metadata: demoDashboard.Metadata,
Spec: struct{}{},
})

return []api.Entity{persesProject, demoDashboard}
})
}

func extractDashboardFromHTTPBody(body interface{}, t *testing.T) *modelV1.Dashboard {
b := testUtils.JSONMarshalStrict(body)
dashboard := &modelV1.Dashboard{}
Expand Down
11 changes: 11 additions & 0 deletions internal/api/impl/v1/dashboard/persistence.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package dashboard
import (
databaseModel "github.com/perses/perses/internal/api/database/model"
"github.com/perses/perses/internal/api/interface/v1/dashboard"
"github.com/perses/perses/pkg/model/api"
v1 "github.com/perses/perses/pkg/model/api/v1"
)

Expand Down Expand Up @@ -58,3 +59,13 @@ func (d *dao) List(q *dashboard.Query) ([]*v1.Dashboard, error) {
err := d.client.Query(q, &result)
return result, err
}

func (d *dao) MetadataList(q *dashboard.Query) ([]api.Entity, error) {
var list []*v1.PartialProjectEntity
err := d.client.Query(q, &list)
result := make([]api.Entity, 0, len(list))
for _, el := range list {
result = append(result, el)
}
return result, err
}
29 changes: 21 additions & 8 deletions internal/api/impl/v1/dashboard/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package dashboard

import (
"fmt"
"github.com/perses/perses/pkg/model/api"

"github.com/brunoga/deep"
apiInterface "github.com/perses/perses/internal/api/interface"
Expand Down Expand Up @@ -113,19 +114,19 @@ func (s *service) Get(_ apiInterface.PersesContext, parameters apiInterface.Para
}

func (s *service) List(_ apiInterface.PersesContext, q *dashboard.Query, params apiInterface.Parameters) ([]*v1.Dashboard, error) {
// Query is copied because it can be modified by the toolbox.go: listWhenPermissionIsActivated(...) and need to `q` need to keep initial value
query, err := deep.Copy(q)
query, err := manageQuery(q, params)
if err != nil {
return nil, fmt.Errorf("unable to copy the query: %w", err)
return nil, err
}
return s.list(query, params)
return s.dao.List(query)
}

func (s *service) list(q *dashboard.Query, params apiInterface.Parameters) ([]*v1.Dashboard, error) {
if len(q.Project) == 0 {
q.Project = params.Project
func (s *service) MetadataList(_ apiInterface.PersesContext, q *dashboard.Query, params apiInterface.Parameters) ([]api.Entity, error) {
query, err := manageQuery(q, params)
if err != nil {
return nil, err
}
return s.dao.List(q)
return s.dao.MetadataList(query)
}

func (s *service) Validate(entity *v1.Dashboard) error {
Expand Down Expand Up @@ -155,3 +156,15 @@ func (s *service) collectProjectVariables(project string) ([]*v1.Variable, error
func (s *service) collectGlobalVariables() ([]*v1.GlobalVariable, error) {
return s.globalVarDAO.List(&globalvariable.Query{})
}

func manageQuery(q *dashboard.Query, params apiInterface.Parameters) (*dashboard.Query, error) {
// Query is copied because it can be modified by the toolbox.go: listWhenPermissionIsActivated(...) and need to `q` need to keep initial value
query, err := deep.Copy(q)
if err != nil {
return nil, fmt.Errorf("unable to copy the query: %w", err)
}
if len(query.Project) == 0 {
query.Project = params.Project
}
return query, nil
}
29 changes: 16 additions & 13 deletions internal/api/impl/v1/datasource/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package datasource

import (
"fmt"

"github.com/brunoga/deep"
apiInterface "github.com/perses/perses/internal/api/interface"
"github.com/perses/perses/internal/api/interface/v1/datasource"
Expand Down Expand Up @@ -101,23 +100,15 @@ func (s *service) Get(_ apiInterface.PersesContext, parameters apiInterface.Para
return s.dao.Get(parameters.Project, parameters.Name)
}
func (s *service) List(_ apiInterface.PersesContext, q *datasource.Query, params apiInterface.Parameters) ([]*v1.Datasource, error) {
// Query is copied because it can be modified by the toolbox.go: listWhenPermissionIsActivated(...) and need to `q` need to keep initial value
query, err := deep.Copy(q)
query, err := manageQuery(q, params)
if err != nil {
return nil, fmt.Errorf("unable to copy the query: %w", err)
}
return s.list(query, params)
}

func (s *service) list(q *datasource.Query, params apiInterface.Parameters) ([]*v1.Datasource, error) {
if len(q.Project) == 0 {
q.Project = params.Project
return nil, err
}
dtsList, err := s.dao.List(q)
dtsList, err := s.dao.List(query)
if err != nil {
return nil, err
}
return v1.FilterDatasource(q.Kind, q.Default, dtsList), nil
return v1.FilterDatasource(query.Kind, query.Default, dtsList), nil
}

func (s *service) validate(entity *v1.Datasource) error {
Expand All @@ -133,3 +124,15 @@ func (s *service) validate(entity *v1.Datasource) error {
}
return validate.Datasource(entity, list, s.sch)
}

func manageQuery(q *datasource.Query, params apiInterface.Parameters) (*datasource.Query, error) {
// Query is copied because it can be modified by the toolbox.go: listWhenPermissionIsActivated(...) and need to `q` need to keep initial value
query, err := deep.Copy(q)
if err != nil {
return nil, fmt.Errorf("unable to copy the query: %w", err)
}
if len(query.Project) == 0 {
query.Project = params.Project
}
return query, nil
}
11 changes: 11 additions & 0 deletions internal/api/impl/v1/ephemeraldashboard/persistence.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package ephemeraldashboard
import (
databaseModel "github.com/perses/perses/internal/api/database/model"
"github.com/perses/perses/internal/api/interface/v1/ephemeraldashboard"
"github.com/perses/perses/pkg/model/api"
v1 "github.com/perses/perses/pkg/model/api/v1"
)

Expand Down Expand Up @@ -58,3 +59,13 @@ func (d *dao) List(q *ephemeraldashboard.Query) ([]*v1.EphemeralDashboard, error
err := d.client.Query(q, &result)
return result, err
}

func (d *dao) MetadataList(q *ephemeraldashboard.Query) ([]api.Entity, error) {
var list []*v1.PartialProjectEntity
err := d.client.Query(q, &list)
result := make([]api.Entity, 0, len(list))
for _, el := range list {
result = append(result, el)
}
return result, err
}
28 changes: 20 additions & 8 deletions internal/api/impl/v1/ephemeraldashboard/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package ephemeraldashboard

import (
"fmt"
"github.com/perses/perses/pkg/model/api"

"github.com/brunoga/deep"
apiInterface "github.com/perses/perses/internal/api/interface"
Expand Down Expand Up @@ -113,19 +114,19 @@ func (s *service) Get(_ apiInterface.PersesContext, parameters apiInterface.Para
}

func (s *service) List(_ apiInterface.PersesContext, q *ephemeraldashboard.Query, params apiInterface.Parameters) ([]*v1.EphemeralDashboard, error) {
// Query is copied because it can be modified by the toolbox.go: listWhenPermissionIsActivated(...) and need to `q` need to keep initial value
query, err := deep.Copy(q)
query, err := manageQuery(q, params)
if err != nil {
return nil, fmt.Errorf("unable to copy the query: %w", err)
return nil, err
}
return s.list(query, params)
return s.dao.List(query)
}

func (s *service) list(q *ephemeraldashboard.Query, params apiInterface.Parameters) ([]*v1.EphemeralDashboard, error) {
if len(q.Project) == 0 {
q.Project = params.Project
func (s *service) MetadataList(_ apiInterface.PersesContext, q *ephemeraldashboard.Query, params apiInterface.Parameters) ([]api.Entity, error) {
query, err := manageQuery(q, params)
if err != nil {
return nil, err
}
return s.dao.List(q)
return s.dao.MetadataList(query)
}

func (s *service) Validate(entity *v1.EphemeralDashboard) error {
Expand Down Expand Up @@ -155,3 +156,14 @@ func (s *service) collectProjectVariables(project string) ([]*v1.Variable, error
func (s *service) collectGlobalVariables() ([]*v1.GlobalVariable, error) {
return s.globalVarDAO.List(&globalvariable.Query{})
}

func manageQuery(q *ephemeraldashboard.Query, params apiInterface.Parameters) (*ephemeraldashboard.Query, error) {
query, err := deep.Copy(q)
if err != nil {
return nil, fmt.Errorf("unable to copy the query: %w", err)
}
if len(query.Project) == 0 {
query.Project = params.Project
}
return query, nil
}
11 changes: 11 additions & 0 deletions internal/api/impl/v1/folder/persistence.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package folder
import (
databaseModel "github.com/perses/perses/internal/api/database/model"
"github.com/perses/perses/internal/api/interface/v1/folder"
"github.com/perses/perses/pkg/model/api"
v1 "github.com/perses/perses/pkg/model/api/v1"
)

Expand Down Expand Up @@ -58,3 +59,13 @@ func (d *dao) List(q *folder.Query) ([]*v1.Folder, error) {
err := d.client.Query(q, &result)
return result, err
}

func (d *dao) MetadataList(q *folder.Query) ([]api.Entity, error) {
var list []*v1.PartialProjectEntity
err := d.client.Query(q, &list)
result := make([]api.Entity, 0, len(list))
for _, el := range list {
result = append(result, el)
}
return result, err
}
28 changes: 20 additions & 8 deletions internal/api/impl/v1/folder/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ package folder

import (
"fmt"

"github.com/brunoga/deep"
apiInterface "github.com/perses/perses/internal/api/interface"
"github.com/perses/perses/internal/api/interface/v1/folder"
"github.com/perses/perses/pkg/model/api"
v1 "github.com/perses/perses/pkg/model/api/v1"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -92,17 +92,29 @@ func (s *service) Get(_ apiInterface.PersesContext, parameters apiInterface.Para
}

func (s *service) List(_ apiInterface.PersesContext, q *folder.Query, params apiInterface.Parameters) ([]*v1.Folder, error) {
query, err := manageQuery(q, params)
if err != nil {
return nil, err
}
return s.dao.List(query)
}

func (s *service) MetadataList(_ apiInterface.PersesContext, q *folder.Query, params apiInterface.Parameters) ([]api.Entity, error) {
query, err := manageQuery(q, params)
if err != nil {
return nil, err
}
return s.dao.MetadataList(query)
}

func manageQuery(q *folder.Query, params apiInterface.Parameters) (*folder.Query, error) {
// Query is copied because it can be modified by the toolbox.go: listWhenPermissionIsActivated(...) and need to `q` need to keep initial value
query, err := deep.Copy(q)
if err != nil {
return nil, fmt.Errorf("unable to copy the query: %w", err)
}
return s.list(query, params)
}

func (s *service) list(q *folder.Query, params apiInterface.Parameters) ([]*v1.Folder, error) {
if len(q.Project) == 0 {
q.Project = params.Project
if len(query.Project) == 0 {
query.Project = params.Project
}
return s.dao.List(q)
return query, nil
}
11 changes: 11 additions & 0 deletions internal/api/impl/v1/globalrole/persistence.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package globalrole
import (
databaseModel "github.com/perses/perses/internal/api/database/model"
"github.com/perses/perses/internal/api/interface/v1/globalrole"
"github.com/perses/perses/pkg/model/api"
v1 "github.com/perses/perses/pkg/model/api/v1"
)

Expand Down Expand Up @@ -54,3 +55,13 @@ func (d *dao) List(q *globalrole.Query) ([]*v1.GlobalRole, error) {
err := d.client.Query(q, &result)
return result, err
}

func (d *dao) MetadataList(q *globalrole.Query) ([]api.Entity, error) {
var list []*v1.PartialEntity
err := d.client.Query(q, &list)
result := make([]api.Entity, 0, len(list))
for _, el := range list {
result = append(result, el)
}
return result, err
}
5 changes: 5 additions & 0 deletions internal/api/impl/v1/globalrole/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package globalrole

import (
"fmt"
"github.com/perses/perses/pkg/model/api"

"github.com/brunoga/deep"
apiInterface "github.com/perses/perses/internal/api/interface"
Expand Down Expand Up @@ -110,3 +111,7 @@ func (s *service) Get(_ apiInterface.PersesContext, parameters apiInterface.Para
func (s *service) List(_ apiInterface.PersesContext, q *globalrole.Query, _ apiInterface.Parameters) ([]*v1.GlobalRole, error) {
return s.dao.List(q)
}

func (s *service) MetadataList(_ apiInterface.PersesContext, q *globalrole.Query, _ apiInterface.Parameters) ([]api.Entity, error) {
return s.dao.MetadataList(q)
}
11 changes: 11 additions & 0 deletions internal/api/impl/v1/globalrolebinding/persistence.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package globalrolebinding
import (
databaseModel "github.com/perses/perses/internal/api/database/model"
"github.com/perses/perses/internal/api/interface/v1/globalrolebinding"
"github.com/perses/perses/pkg/model/api"
v1 "github.com/perses/perses/pkg/model/api/v1"
)

Expand Down Expand Up @@ -54,3 +55,13 @@ func (d *dao) List(q *globalrolebinding.Query) ([]*v1.GlobalRoleBinding, error)
err := d.client.Query(q, &result)
return result, err
}

func (d *dao) MetadataList(q *globalrolebinding.Query) ([]api.Entity, error) {
var list []*v1.PartialEntity
err := d.client.Query(q, &list)
result := make([]api.Entity, 0, len(list))
for _, el := range list {
result = append(result, el)
}
return result, err
}
Loading
Loading