fix: implement sort parameter for workflows query

Signed-off-by: Michael Adler <[email protected]>
siemens · Aug 19, 2024 · 675276d · 675276d
1 parent 8256726
commit 675276d
Show file tree

Hide file tree

Showing 16 changed files with 83 additions and 47 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - `wfx` would not start if it was built without plugins support
+- `wfx`: implemented sort functionality for `/workflows` endpoint
 
 ### Changed
 

diff --git a/api/southbound_test.go b/api/southbound_test.go
@@ -255,7 +255,7 @@ func TestGetWorkflows_InternalError(t *testing.T) {
 	for _, orientation := range allOrientations {
 		t.Run(orientation, func(t *testing.T) {
 			dbMock := persistence.NewHealthyMockStorage(t)
-			dbMock.EXPECT().QueryWorkflows(context.Background(), persistence.PaginationParams{Limit: 10}).Return(nil, errors.New("something went wrong"))
+			dbMock.EXPECT().QueryWorkflows(context.Background(), persistence.SortParams{}, persistence.PaginationParams{Limit: 10}).Return(nil, errors.New("something went wrong"))
 
 			server := createServerForTesting(t, orientation, dbMock)
 			resp, err := server.GetWorkflows(context.Background(), api.GetWorkflowsRequestObject{Params: api.GetWorkflowsParams{}})
@@ -269,7 +269,7 @@ func TestGetWorkflows_Empty(t *testing.T) {
 	for _, orientation := range allOrientations {
 		t.Run(orientation, func(t *testing.T) {
 			dbMock := persistence.NewHealthyMockStorage(t)
-			dbMock.EXPECT().QueryWorkflows(context.Background(), persistence.PaginationParams{Limit: 10}).Return(&api.PaginatedWorkflowList{}, nil)
+			dbMock.EXPECT().QueryWorkflows(context.Background(), persistence.SortParams{Desc: false}, persistence.PaginationParams{Limit: 10}).Return(&api.PaginatedWorkflowList{}, nil)
 
 			server := createServerForTesting(t, orientation, dbMock)
 			resp, err := server.GetWorkflows(context.Background(), api.GetWorkflowsRequestObject{Params: api.GetWorkflowsParams{}})

diff --git a/api/wfx.go b/api/wfx.go
@@ -328,7 +328,7 @@ func (server WfxServer) GetWorkflows(ctx context.Context, request api.GetWorkflo
 	}
 	pagination := persistence.PaginationParams{Offset: offset, Limit: limit}
 	log := logging.LoggerFromCtx(ctx)
-	workflows, err := workflow.QueryWorkflows(ctx, server.storage, pagination)
+	workflows, err := workflow.QueryWorkflows(ctx, server.storage, pagination, (*string)(request.Params.ParamSort))
 	if err != nil {
 		log.Error().Err(err).Msg("Failed to query workflows")
 		return nil, fault.Wrap(err)

diff --git a/api/workflow_test.go b/api/workflow_test.go
@@ -186,7 +186,7 @@ func newInMemoryDB(t *testing.T) persistence.Storage {
 			}
 		}
 		{
-			list, _ := db.QueryWorkflows(context.Background(), persistence.PaginationParams{Limit: 100})
+			list, _ := db.QueryWorkflows(context.Background(), persistence.SortParams{Desc: false}, persistence.PaginationParams{Limit: 100})
 			for _, wf := range list.Content {
 				_ = db.DeleteWorkflow(context.Background(), wf.Name)
 			}

diff --git a/internal/handler/job/create_test.go b/internal/handler/job/create_test.go
@@ -70,7 +70,7 @@ func newInMemoryDB(t *testing.T) persistence.Storage {
 			}
 		}
 		{
-			list, _ := db.QueryWorkflows(context.Background(), persistence.PaginationParams{Limit: 100})
+			list, _ := db.QueryWorkflows(context.Background(), persistence.SortParams{}, persistence.PaginationParams{Limit: 100})
 			for _, wf := range list.Content {
 				_ = db.DeleteWorkflow(context.Background(), wf.Name)
 			}

diff --git a/internal/handler/job/definition/get_test.go b/internal/handler/job/definition/get_test.go
@@ -62,7 +62,7 @@ func newInMemoryDB(t *testing.T) persistence.Storage {
 			}
 		}
 		{
-			list, _ := db.QueryWorkflows(context.Background(), persistence.PaginationParams{Limit: 100})
+			list, _ := db.QueryWorkflows(context.Background(), persistence.SortParams{}, persistence.PaginationParams{Limit: 100})
 			for _, wf := range list.Content {
 				_ = db.DeleteWorkflow(context.Background(), wf.Name)
 			}

diff --git a/internal/handler/job/status/get_test.go b/internal/handler/job/status/get_test.go
@@ -65,7 +65,7 @@ func newInMemoryDB(t *testing.T) persistence.Storage {
 			}
 		}
 		{
-			list, _ := db.QueryWorkflows(context.Background(), persistence.PaginationParams{Limit: 100})
+			list, _ := db.QueryWorkflows(context.Background(), persistence.SortParams{Desc: false}, persistence.PaginationParams{Limit: 100})
 			for _, wf := range list.Content {
 				_ = db.DeleteWorkflow(context.Background(), wf.Name)
 			}

diff --git a/internal/handler/job/tags/add_test.go b/internal/handler/job/tags/add_test.go
@@ -94,7 +94,7 @@ func newInMemoryDB(t *testing.T) persistence.Storage {
 			}
 		}
 		{
-			list, _ := db.QueryWorkflows(context.Background(), persistence.PaginationParams{Limit: 100})
+			list, _ := db.QueryWorkflows(context.Background(), persistence.SortParams{Desc: false}, persistence.PaginationParams{Limit: 100})
 			for _, wf := range list.Content {
 				_ = db.DeleteWorkflow(context.Background(), wf.Name)
 			}

diff --git a/internal/handler/workflow/query.go b/internal/handler/workflow/query.go
@@ -10,16 +10,21 @@ package workflow
 
 import (
 	"context"
+	"strings"
 
 	"github.com/Southclaws/fault"
 	"github.com/siemens/wfx/generated/api"
 	"github.com/siemens/wfx/middleware/logging"
 	"github.com/siemens/wfx/persistence"
 )
 
-func QueryWorkflows(ctx context.Context, storage persistence.Storage, paginationParams persistence.PaginationParams) (*api.PaginatedWorkflowList, error) {
+func QueryWorkflows(ctx context.Context, storage persistence.Storage, paginationParams persistence.PaginationParams, sort *string) (*api.PaginatedWorkflowList, error) {
 	log := logging.LoggerFromCtx(ctx)
-	log.Debug().Msg("Querying workflows")
-	list, err := storage.QueryWorkflows(ctx, paginationParams)
+	var sortParams persistence.SortParams
+	if sort != nil {
+		sortParams.Desc = strings.ToLower(*sort) == "desc"
+	}
+	log.Debug().Bool("desc", sortParams.Desc).Msg("Querying workflows")
+	list, err := storage.QueryWorkflows(ctx, sortParams, paginationParams)
 	return list, fault.Wrap(err)
 }
diff --git a/internal/handler/workflow/query_test.go b/internal/handler/workflow/query_test.go
@@ -23,7 +23,7 @@ func TestQueryWorkflow(t *testing.T) {
 	wf, err := CreateWorkflow(context.Background(), db, dau.DirectWorkflow())
 	require.NoError(t, err)
 
-	list, err := QueryWorkflows(context.Background(), db, persistence.PaginationParams{Limit: 10})
+	list, err := QueryWorkflows(context.Background(), db, persistence.PaginationParams{Limit: 10}, nil)
 	assert.NoError(t, err)
 	assert.Len(t, list.Content, 1)
 	assert.Equal(t, wf.Name, list.Content[0].Name)

diff --git a/internal/persistence/entgo/sqlite_test.go b/internal/persistence/entgo/sqlite_test.go
@@ -25,11 +25,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestSQLite_Initialize(t *testing.T) {
-	db := setupSQLite(t)
-	db.Shutdown()
-}
-
 func TestSQLite(t *testing.T) {
 	db := setupSQLite(t)
 	t.Cleanup(db.Shutdown)

diff --git a/internal/persistence/entgo/workflow_query.go b/internal/persistence/entgo/workflow_query.go
@@ -20,19 +20,28 @@ import (
 )
 
 // QueryWorkflows returns multiple workflows (paginated).
-func (db Database) QueryWorkflows(ctx context.Context, paginationParams persistence.PaginationParams) (*api.PaginatedWorkflowList, error) {
+func (db Database) QueryWorkflows(ctx context.Context, sortParams persistence.SortParams, paginationParams persistence.PaginationParams) (*api.PaginatedWorkflowList, error) {
 	log := logging.LoggerFromCtx(ctx)
 	builder := db.client.Workflow.
 		Query()
 
 	// need to clone builder because it is unusable after we call `All`
 	counter := builder.Clone()
 
-	workflows, err := builder.
+	builder.
 		Limit(int(paginationParams.Limit)).
-		Offset(int(paginationParams.Offset)).
-		Order(ent.Asc(workflow.FieldName)).
-		All(ctx)
+		Offset(int(paginationParams.Offset))
+
+	// deterministic ordering
+	if sortParams.Desc {
+		log.Debug().Msg("Sorting workflows in descending order")
+		builder.Order(ent.Desc(workflow.FieldName))
+	} else {
+		log.Debug().Msg("Sorting workflows in ascending order")
+		builder.Order(ent.Asc(workflow.FieldName))
+	}
+
+	workflows, err := builder.All(ctx)
 	if err != nil {
 		return nil, fault.Wrap(err)
 	}

diff --git a/internal/persistence/tests/all.go b/internal/persistence/tests/all.go
@@ -29,4 +29,5 @@ var AllTests = []PersistenceTest{
 	TestUpdateJobStatus,
 	TestUpdateJobStatusNonExisting,
 	TestWorkflowsPagination,
+	TestQueryWorkflowsSort,
 }
diff --git a/internal/persistence/tests/workflow.go b/internal/persistence/tests/workflow.go
@@ -70,15 +70,9 @@ func TestQueryWorkflows(t *testing.T, db persistence.Storage) {
 	require.NoError(t, err)
 
 	allCount := 1
-	result, err := db.QueryWorkflows(context.Background(), persistence.PaginationParams{Offset: 0, Limit: int32(allCount)})
+	result, err := db.QueryWorkflows(context.Background(), persistence.SortParams{}, persistence.PaginationParams{Offset: 0, Limit: int32(allCount)})
 	require.NoError(t, err)
 	assert.Len(t, result.Content, allCount)
-
-	keys := make([]string, 0, len(result.Content))
-	for _, workflow := range result.Content {
-		keys = append(keys, workflow.Name)
-	}
-	assert.IsIncreasing(t, keys)
 }
 
 func TestWorkflowsPagination(t *testing.T, db persistence.Storage) {
@@ -87,16 +81,46 @@ func TestWorkflowsPagination(t *testing.T, db persistence.Storage) {
 	_, err = db.CreateWorkflow(context.Background(), dau.PhasedWorkflow())
 	require.NoError(t, err)
 
-	result, err := db.QueryWorkflows(context.Background(), persistence.PaginationParams{Offset: 0, Limit: 1})
+	result, err := db.QueryWorkflows(context.Background(), persistence.SortParams{}, persistence.PaginationParams{Offset: 0, Limit: 1})
 	assert.NoError(t, err)
 	assert.Len(t, result.Content, 1)
 
-	result2, err := db.QueryWorkflows(context.Background(), persistence.PaginationParams{Offset: 1, Limit: 1})
+	result2, err := db.QueryWorkflows(context.Background(), persistence.SortParams{}, persistence.PaginationParams{Offset: 1, Limit: 1})
 	assert.NoError(t, err)
 	assert.Len(t, result2.Content, 1)
 	assert.NotEqual(t, result.Content[0].Name, result2.Content[0].Name)
 
-	result3, err := db.QueryWorkflows(context.Background(), persistence.PaginationParams{Offset: 2, Limit: 1})
+	result3, err := db.QueryWorkflows(context.Background(), persistence.SortParams{}, persistence.PaginationParams{Offset: 2, Limit: 1})
 	assert.NoError(t, err)
 	assert.Len(t, result3.Content, 0)
 }
+
+func TestQueryWorkflowsSort(t *testing.T, db persistence.Storage) {
+	_, _ = db.CreateWorkflow(context.Background(), dau.DirectWorkflow())
+	_, _ = db.CreateWorkflow(context.Background(), dau.PhasedWorkflow())
+
+	allCount := 2
+	t.Run("asc", func(t *testing.T) {
+		result, err := db.QueryWorkflows(context.Background(), persistence.SortParams{Desc: false}, persistence.PaginationParams{Offset: 0, Limit: int32(allCount)})
+		require.NoError(t, err)
+		assert.Len(t, result.Content, allCount)
+
+		keys := make([]string, 0, len(result.Content))
+		for _, workflow := range result.Content {
+			keys = append(keys, workflow.Name)
+		}
+		assert.IsIncreasing(t, keys)
+	})
+
+	t.Run("desc", func(t *testing.T) {
+		result, err := db.QueryWorkflows(context.Background(), persistence.SortParams{Desc: true}, persistence.PaginationParams{Offset: 0, Limit: int32(allCount)})
+		require.NoError(t, err)
+		assert.Len(t, result.Content, allCount)
+
+		keys := make([]string, 0, len(result.Content))
+		for _, workflow := range result.Content {
+			keys = append(keys, workflow.Name)
+		}
+		assert.IsDecreasing(t, keys)
+	})
+}
diff --git a/persistence/mock_Storage.go b/persistence/mock_Storage.go
diff --git a/persistence/storage.go b/persistence/storage.go
@@ -53,7 +53,7 @@ type Storage interface {
 	DeleteWorkflow(ctx context.Context, name string) error
 
 	// QueryWorkflows retrieves all workflows from the storage respecting the paginationParams.
-	QueryWorkflows(ctx context.Context, paginationParams PaginationParams) (*api.PaginatedWorkflowList, error)
+	QueryWorkflows(ctx context.Context, sortParams SortParams, paginationParams PaginationParams) (*api.PaginatedWorkflowList, error)
 }
 
 // JobUpdate encapsulates the properties of a job that can be updated.