From c4edaab3f2cb8ff1fb5787ee36a86a15095fd3ea Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Tue, 13 Aug 2024 13:07:20 -0500 Subject: [PATCH 01/34] feat(arangodb/statement.go): add new AQL queries for content listing with sorting and pagination Added multiple AQL queries to handle content listing with various filters and pagination options in the ArangoDB repository layer. These queries support sorting by creation date, limiting the number of results, and using a cursor for pagination. This enhancement allows for more efficient data retrieval and supports building more responsive and scalable applications. --- internal/repository/arangodb/statement.go | 29 ++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/internal/repository/arangodb/statement.go b/internal/repository/arangodb/statement.go index 4810f72..bb6062c 100644 --- a/internal/repository/arangodb/statement.go +++ b/internal/repository/arangodb/statement.go @@ -7,7 +7,34 @@ const ( LIMIT 1 RETURN cnt ` - + ContentList = ` + FOR cnt IN @@content_collection + SORT cnt.created_on DESC + LIMIT @limit + RETURN cnt + ` + ContentListWithCursor = ` + FOR cnt IN @@content_collection + FILTER cnt.created_on <= DATE_ISO8601(@cursor) + SORT cnt.created_on DESC + LIMIT @limit + RETURN cnt + ` + ContentListFilter = ` + FOR cnt IN @@content_collection + %s + SORT cnt.created_on DESC + LIMIT @limit + RETURN cnt + ` + ContentListFilterWithCursor = ` + FOR cnt IN @@content_collection + FILTER cnt.created_on <= DATE_ISO8601(@cursor) + %s + SORT cnt.created_on DESC + LIMIT @limit + RETURN cnt + ` ContentInsert = ` INSERT { name: @name, From 5a53cc60d0e5058a40da5197567c500eef351f36 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Tue, 13 Aug 2024 13:11:46 -0500 Subject: [PATCH 02/34] feat(arangodb.go): add getListContentStatement to handle content retrieval with filters and cursor This function enhances the content retrieval process by dynamically generating AQL statements based on the presence of a filter and cursor. It allows for more efficient data fetching, especially in scenarios where pagination or specific filtering is required. --- internal/repository/arangodb/arangodb.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/repository/arangodb/arangodb.go b/internal/repository/arangodb/arangodb.go index 2b87a47..6d6b48a 100644 --- a/internal/repository/arangodb/arangodb.go +++ b/internal/repository/arangodb/arangodb.go @@ -216,3 +216,19 @@ func (arp *arangorepository) EditContent( func (arp *arangorepository) Dbh() *manager.Database { return arp.database } + +func getListContentStatement(filter string, cursor int64) string { + var stmt string + switch { + case len(filter) > 0 && cursor == 0: + stmt = fmt.Sprintf(ContentListFilter, filter) + case len(filter) > 0 && cursor != 0: + stmt = fmt.Sprintf(ContentListFilterWithCursor, filter) + case len(filter) == 0 && cursor == 0: + stmt = ContentList + case len(filter) == 0 && cursor != 0: + stmt = ContentListWithCursor + } + + return stmt +} From ccbb4b9d66714cc1f84aeeb0981aad347d4a481c Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Tue, 13 Aug 2024 16:09:31 -0500 Subject: [PATCH 03/34] feat(repository.go): add ListContents method and ContentListNotFoundError to ContentRepository The `ListContents` method is introduced to the `ContentRepository` interface to support fetching lists of content based on pagination and sorting parameters. Additionally, the `ContentListNotFoundError` type and the `IsContentListNotFound` function are added to handle scenarios where a content list query returns no results. --- internal/repository/repository.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 4cb5c0f..f7a6ad9 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -15,5 +15,20 @@ type ContentRepository interface { cnt *content.ExistingContentAttributes, ) (*model.ContentDoc, error) DeleteContent(cid int64) error + ListContents(int64, int64, string) (*[]model.ContentDoc, error) Dbh() *manager.Database } + +type ContentListNotFoundError struct{} + +func (al *ContentListNotFoundError) Error() string { + return "annotation list not found" +} + +func IsContentListNotFound(err error) bool { + if _, ok := err.(*ContentListNotFoundError); ok { + return true + } + + return false +} From 981d5dc9c60e51c3e9cd21cf227f61498f3e892d Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Tue, 13 Aug 2024 16:10:09 -0500 Subject: [PATCH 04/34] feat(arangodb.go): add ListContents method to fetch paginated content with filters The new `ListContents` method in the ArangoDB repository allows fetching content documents in a paginated manner, which is crucial for handling large datasets efficiently. It supports cursor-based pagination and filtering, enhancing the application's ability to manage and display large amounts of data dynamically based on user inputs or internal criteria. --- internal/repository/arangodb/arangodb.go | 36 ++++++++++++++++++++++++ internal/repository/repository.go | 6 +++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/internal/repository/arangodb/arangodb.go b/internal/repository/arangodb/arangodb.go index 6d6b48a..55672f0 100644 --- a/internal/repository/arangodb/arangodb.go +++ b/internal/repository/arangodb/arangodb.go @@ -217,6 +217,42 @@ func (arp *arangorepository) Dbh() *manager.Database { return arp.database } +func (arp *arangorepository) ListContents( + cursor int64, + limit int64, + filter string, +) ([]*model.ContentDoc, error) { + bindVars := map[string]interface{}{ + "@content_collection": arp.content.Name(), + "limit": limit + 1, + } + if cursor != 0 { + bindVars["cursor"] = cursor + } + stmt := getListContentStatement(filter, cursor) + res, err := arp.database.SearchRows(stmt, bindVars) + if err != nil { + return nil, fmt.Errorf("error in searching rows %s", err) + } + if res.IsEmpty() { + return nil, &repository.ContentListNotFoundError{} + } + + contentModel := make([]*model.ContentDoc, 0) + for res.Scan() { + m := &model.ContentDoc{} + if err := res.Read(m); err != nil { + return nil, fmt.Errorf( + "error in reading data to structure %s", + err, + ) + } + contentModel = append(contentModel, m) + } + + return contentModel, nil +} + func getListContentStatement(filter string, cursor int64) string { var stmt string switch { diff --git a/internal/repository/repository.go b/internal/repository/repository.go index f7a6ad9..b8a4b9f 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -15,7 +15,11 @@ type ContentRepository interface { cnt *content.ExistingContentAttributes, ) (*model.ContentDoc, error) DeleteContent(cid int64) error - ListContents(int64, int64, string) (*[]model.ContentDoc, error) + ListContents( + cursor int64, + limit int64, + filter string, + ) ([]*model.ContentDoc, error) Dbh() *manager.Database } From 32ddc743871aef1c38de88983a1c39fe434ebb67 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Tue, 13 Aug 2024 16:59:16 -0500 Subject: [PATCH 05/34] feat(arangodb_test.go): add TestListContents to validate content listing functionality The addition of the `TestListContents` function in the `arangodb_test.go` file enhances the test coverage by ensuring the `ListContents` method behaves as expected under various scenarios. This includes verifying the correct number of contents returned, the namespace accuracy, and the order of content creation. The inclusion of new imports (`fmt` and `regexp`) supports the formatting and pattern matching required for the tests. --- internal/repository/arangodb/arangodb_test.go | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/internal/repository/arangodb/arangodb_test.go b/internal/repository/arangodb/arangodb_test.go index aab13cd..4bd34b9 100644 --- a/internal/repository/arangodb/arangodb_test.go +++ b/internal/repository/arangodb/arangodb_test.go @@ -2,6 +2,8 @@ package arangodb import ( "encoding/json" + "fmt" + "regexp" "strconv" "testing" "time" @@ -87,6 +89,42 @@ func TestGetContentBySlug(t *testing.T) { testContentProperties(assert, sct, nct) } +func TestListContents(t *testing.T) { + t.Parallel() + assert, repo := setUp(t) + defer tearDown(repo) + for i := 0; i < 19; i++ { + _, err := repo.AddContent( + testutils.NewStoreContent(fmt.Sprintf("gallery-%d", i), "genome"), + ) + assert.NoErrorf( + err, + "expect no error from creating gallery genome content %s", + err, + ) + } + clist, err := repo.ListContents(0, 4, "") + assert.NoError(err, "expect no error from listing content") + assert.Len(clist, 5, "should have 5 content entries") + nrxp := regexp.MustCompile(`gallery-\d+`) + for _, cnt := range clist { + assert.Equal( + cnt.Namespace, + "genome", + "should match the genome namespace", + ) + assert.Regexp(nrxp, cnt.Name, "should match gallery names") + } + assert.True( + clist[0].CreatedOn.After(clist[1].CreatedOn), + "first gallery record should be created after the second one", + ) + assert.False( + clist[2].CreatedOn.After(clist[1].CreatedOn), + "third gallery record should not be created after the second one", + ) +} + func TestGetContent(t *testing.T) { t.Parallel() assert, repo := setUp(t) From 2c1437b924ed2780b38e5a30c6db131b61271106 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Tue, 13 Aug 2024 19:18:22 -0500 Subject: [PATCH 06/34] test(arangodb_test.go): reduce test data size and enhance content listing tests The test data size in `TestListContents` is reduced from 19 to 14 to make the test execution faster and more focused. Additionally, the test now includes checks for the `CreatedBy` field to ensure data integrity and a loop to verify the chronological order of content creation, enhancing the robustness of the test. New assertions are added to validate the continuation of content listing through pagination, ensuring that subsequent pages correctly follow from the last entry of the previous page. This helps in verifying the functionality of content listing over multiple pages. --- internal/repository/arangodb/arangodb_test.go | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/internal/repository/arangodb/arangodb_test.go b/internal/repository/arangodb/arangodb_test.go index 4bd34b9..09b2ec5 100644 --- a/internal/repository/arangodb/arangodb_test.go +++ b/internal/repository/arangodb/arangodb_test.go @@ -93,7 +93,7 @@ func TestListContents(t *testing.T) { t.Parallel() assert, repo := setUp(t) defer tearDown(repo) - for i := 0; i < 19; i++ { + for i := 0; i < 14; i++ { _, err := repo.AddContent( testutils.NewStoreContent(fmt.Sprintf("gallery-%d", i), "genome"), ) @@ -107,21 +107,46 @@ func TestListContents(t *testing.T) { assert.NoError(err, "expect no error from listing content") assert.Len(clist, 5, "should have 5 content entries") nrxp := regexp.MustCompile(`gallery-\d+`) - for _, cnt := range clist { + for idx, cnt := range clist { assert.Equal( cnt.Namespace, "genome", "should match the genome namespace", ) assert.Regexp(nrxp, cnt.Name, "should match gallery names") + assert.Equal( + cnt.CreatedBy, "content@content.org", "should match the created_by", + ) + if idx != 0 { + assert.True( + clist[idx-1].CreatedOn.After(clist[idx].CreatedOn), + "previous gallery record should be created after the current one", + ) + } } - assert.True( - clist[0].CreatedOn.After(clist[1].CreatedOn), - "first gallery record should be created after the second one", + clist2, err := repo.ListContents( + clist[len(clist)-1].CreatedOn.UnixMilli(), + 5, + "", ) - assert.False( - clist[2].CreatedOn.After(clist[1].CreatedOn), - "third gallery record should not be created after the second one", + assert.NoError(err, "expect no error from listing content") + assert.Len(clist2, 6, "should have 6 content entries") + assert.Exactly( + clist[len(clist)-1], + clist2[0], + "should be identical content object", + ) + clist3, err := repo.ListContents( + clist2[len(clist2)-1].CreatedOn.UnixMilli(), + 7, + "", + ) + assert.NoError(err, "expect no error from listing content") + assert.Len(clist3, 5, "should have 4 content entries") + assert.Exactly( + clist2[len(clist2)-1], + clist3[0], + "should be identical content object", ) } From 91aba76094ac4b91b52550b57cbea1537540f130 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Tue, 13 Aug 2024 19:23:51 -0500 Subject: [PATCH 07/34] refactor(arangodb_test.go): extract content creation and validation into separate functions Extracting repetitive content creation and validation logic into separate functions `createTestContents` and `validateContentList` improves code readability and maintainability. --- internal/repository/arangodb/arangodb_test.go | 83 ++++++++++++------- 1 file changed, 54 insertions(+), 29 deletions(-) diff --git a/internal/repository/arangodb/arangodb_test.go b/internal/repository/arangodb/arangodb_test.go index 09b2ec5..42d2c38 100644 --- a/internal/repository/arangodb/arangodb_test.go +++ b/internal/repository/arangodb/arangodb_test.go @@ -93,37 +93,17 @@ func TestListContents(t *testing.T) { t.Parallel() assert, repo := setUp(t) defer tearDown(repo) - for i := 0; i < 14; i++ { - _, err := repo.AddContent( - testutils.NewStoreContent(fmt.Sprintf("gallery-%d", i), "genome"), - ) - assert.NoErrorf( - err, - "expect no error from creating gallery genome content %s", - err, - ) - } + + // Create test data + createTestContents(assert, repo, 14) + + // Test initial list clist, err := repo.ListContents(0, 4, "") assert.NoError(err, "expect no error from listing content") assert.Len(clist, 5, "should have 5 content entries") - nrxp := regexp.MustCompile(`gallery-\d+`) - for idx, cnt := range clist { - assert.Equal( - cnt.Namespace, - "genome", - "should match the genome namespace", - ) - assert.Regexp(nrxp, cnt.Name, "should match gallery names") - assert.Equal( - cnt.CreatedBy, "content@content.org", "should match the created_by", - ) - if idx != 0 { - assert.True( - clist[idx-1].CreatedOn.After(clist[idx].CreatedOn), - "previous gallery record should be created after the current one", - ) - } - } + validateContentList(assert, clist) + + // Test pagination clist2, err := repo.ListContents( clist[len(clist)-1].CreatedOn.UnixMilli(), 5, @@ -136,13 +116,15 @@ func TestListContents(t *testing.T) { clist2[0], "should be identical content object", ) + + // Test final page clist3, err := repo.ListContents( clist2[len(clist2)-1].CreatedOn.UnixMilli(), 7, "", ) assert.NoError(err, "expect no error from listing content") - assert.Len(clist3, 5, "should have 4 content entries") + assert.Len(clist3, 5, "should have 5 content entries") assert.Exactly( clist2[len(clist2)-1], clist3[0], @@ -150,6 +132,49 @@ func TestListContents(t *testing.T) { ) } +func createTestContents( + assert *require.Assertions, + repo repository.ContentRepository, + count int, +) { + for i := 0; i < count; i++ { + _, err := repo.AddContent( + testutils.NewStoreContent(fmt.Sprintf("gallery-%d", i), "genome"), + ) + assert.NoErrorf( + err, + "expect no error from creating gallery genome content %s", + err, + ) + } +} + +func validateContentList( + assert *require.Assertions, + clist []*model.ContentDoc, +) { + nrxp := regexp.MustCompile(`gallery-\d+`) + for idx, cnt := range clist { + assert.Equal( + cnt.Namespace, + "genome", + "should match the genome namespace", + ) + assert.Regexp(nrxp, cnt.Name, "should match gallery names") + assert.Equal( + cnt.CreatedBy, + "content@content.org", + "should match the created_by", + ) + if idx != 0 { + assert.True( + clist[idx-1].CreatedOn.After(clist[idx].CreatedOn), + "previous gallery record should be created after the current one", + ) + } + } +} + func TestGetContent(t *testing.T) { t.Parallel() assert, repo := setUp(t) From 43501e6dd0c83bd087a63320f89a546a094052d9 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Wed, 14 Aug 2024 12:30:13 -0500 Subject: [PATCH 08/34] chore(.gitignore): add .plandex to ignore list to prevent tracking of project-specific files The addition of `.plandex` to the `.gitignore` file ensures that project-specific files, which are not relevant to the repository's codebase, are not tracked by Git. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 48b8bf9..a67498b 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ vendor/ +.plandex From 96c4dd82f20565dbdd6645178d0692286b451efc Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Wed, 14 Aug 2024 12:54:02 -0500 Subject: [PATCH 09/34] refactor(arangodb_test.go): move ListContents tests to separate file The content listing tests, including pagination and validation, are moved from `arangodb_test.go` to this new file, ensuring that each test file has a clear and focused responsibility. --- .../repository/arangodb/arangodb_list_test.go | 102 ++++++++++++++++++ internal/repository/arangodb/arangodb_test.go | 88 --------------- 2 files changed, 102 insertions(+), 88 deletions(-) create mode 100644 internal/repository/arangodb/arangodb_list_test.go diff --git a/internal/repository/arangodb/arangodb_list_test.go b/internal/repository/arangodb/arangodb_list_test.go new file mode 100644 index 0000000..aff0629 --- /dev/null +++ b/internal/repository/arangodb/arangodb_list_test.go @@ -0,0 +1,102 @@ +package arangodb + +import ( + "fmt" + "regexp" + "testing" + "time" + + "github.com/dictyBase/modware-content/internal/model" + "github.com/dictyBase/modware-content/internal/repository" + "github.com/dictyBase/modware-content/internal/testutils" + "github.com/stretchr/testify/require" +) + +func TestListContents(t *testing.T) { + t.Parallel() + assert, repo := setUp(t) + defer tearDown(repo) + + // Create test data + createTestContents(assert, repo, 14) + + // Test initial list + clist, err := repo.ListContents(0, 4, "") + assert.NoError(err, "expect no error from listing content") + assert.Len(clist, 5, "should have 5 content entries") + validateContentList(assert, clist) + + // Test pagination + clist2, err := repo.ListContents( + clist[len(clist)-1].CreatedOn.UnixMilli(), + 5, + "", + ) + assert.NoError(err, "expect no error from listing content") + assert.Len(clist2, 6, "should have 6 content entries") + assert.Exactly( + clist[len(clist)-1], + clist2[0], + "should be identical content object", + ) + + // Test final page + clist3, err := repo.ListContents( + clist2[len(clist2)-1].CreatedOn.UnixMilli(), + 7, + "", + ) + assert.NoError(err, "expect no error from listing content") + assert.Len(clist3, 5, "should have 5 content entries") + assert.Exactly( + clist2[len(clist2)-1], + clist3[0], + "should be identical content object", + ) +} + +func createTestContents( + assert *require.Assertions, + repo repository.ContentRepository, + count int, +) { + for i := 0; i < count; i++ { + time.Sleep(1 * time.Millisecond) + _, err := repo.AddContent( + testutils.NewStoreContent(fmt.Sprintf("gallery-%d", i), "genome"), + ) + assert.NoErrorf( + err, + "expect no error from creating gallery genome content %s", + err, + ) + } +} + +func validateContentList( + assert *require.Assertions, + clist []*model.ContentDoc, +) { + nrxp := regexp.MustCompile(`gallery-\d+`) + for idx, cnt := range clist { + assert.Equal( + cnt.Namespace, + "genome", + "should match the genome namespace", + ) + assert.Regexp(nrxp, cnt.Name, "should match gallery names") + assert.Equal( + cnt.CreatedBy, + "content@content.org", + "should match the created_by", + ) + if idx != 0 { + assert.Truef( + clist[idx-1].CreatedOn.After(clist[idx].CreatedOn), + "previous gallery record %s should be created after the current one %s", + clist[idx-1].CreatedOn.String(), + clist[idx].CreatedOn, + ) + } + } +} diff --git a/internal/repository/arangodb/arangodb_test.go b/internal/repository/arangodb/arangodb_test.go index 42d2c38..aab13cd 100644 --- a/internal/repository/arangodb/arangodb_test.go +++ b/internal/repository/arangodb/arangodb_test.go @@ -2,8 +2,6 @@ package arangodb import ( "encoding/json" - "fmt" - "regexp" "strconv" "testing" "time" @@ -89,92 +87,6 @@ func TestGetContentBySlug(t *testing.T) { testContentProperties(assert, sct, nct) } -func TestListContents(t *testing.T) { - t.Parallel() - assert, repo := setUp(t) - defer tearDown(repo) - - // Create test data - createTestContents(assert, repo, 14) - - // Test initial list - clist, err := repo.ListContents(0, 4, "") - assert.NoError(err, "expect no error from listing content") - assert.Len(clist, 5, "should have 5 content entries") - validateContentList(assert, clist) - - // Test pagination - clist2, err := repo.ListContents( - clist[len(clist)-1].CreatedOn.UnixMilli(), - 5, - "", - ) - assert.NoError(err, "expect no error from listing content") - assert.Len(clist2, 6, "should have 6 content entries") - assert.Exactly( - clist[len(clist)-1], - clist2[0], - "should be identical content object", - ) - - // Test final page - clist3, err := repo.ListContents( - clist2[len(clist2)-1].CreatedOn.UnixMilli(), - 7, - "", - ) - assert.NoError(err, "expect no error from listing content") - assert.Len(clist3, 5, "should have 5 content entries") - assert.Exactly( - clist2[len(clist2)-1], - clist3[0], - "should be identical content object", - ) -} - -func createTestContents( - assert *require.Assertions, - repo repository.ContentRepository, - count int, -) { - for i := 0; i < count; i++ { - _, err := repo.AddContent( - testutils.NewStoreContent(fmt.Sprintf("gallery-%d", i), "genome"), - ) - assert.NoErrorf( - err, - "expect no error from creating gallery genome content %s", - err, - ) - } -} - -func validateContentList( - assert *require.Assertions, - clist []*model.ContentDoc, -) { - nrxp := regexp.MustCompile(`gallery-\d+`) - for idx, cnt := range clist { - assert.Equal( - cnt.Namespace, - "genome", - "should match the genome namespace", - ) - assert.Regexp(nrxp, cnt.Name, "should match gallery names") - assert.Equal( - cnt.CreatedBy, - "content@content.org", - "should match the created_by", - ) - if idx != 0 { - assert.True( - clist[idx-1].CreatedOn.After(clist[idx].CreatedOn), - "previous gallery record should be created after the current one", - ) - } - } -} - func TestGetContent(t *testing.T) { t.Parallel() assert, repo := setUp(t) From cf02284e01dcc655085b3055604d58fc3033b112 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Wed, 14 Aug 2024 13:24:32 -0500 Subject: [PATCH 10/34] feat(arangodb_list_test.go): add createCustomTestContents function to generate custom test content The addition of the `createCustomTestContents` function in the test suite allows for more flexible generation of test content based on custom parameters such as `name` and `namespace`. This function helps in creating multiple content items dynamically during testing, which is useful for scenarios requiring varied data setups. --- .../repository/arangodb/arangodb_list_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/internal/repository/arangodb/arangodb_list_test.go b/internal/repository/arangodb/arangodb_list_test.go index aff0629..1ca068a 100644 --- a/internal/repository/arangodb/arangodb_list_test.go +++ b/internal/repository/arangodb/arangodb_list_test.go @@ -73,6 +73,24 @@ func createTestContents( } } +func createCustomTestContents( + assert *require.Assertions, + repo repository.ContentRepository, + count int, + name, namespace string, +) { + for i := 0; i < count; i++ { + time.Sleep(1 * time.Millisecond) + _, err := repo.AddContent( + testutils.NewStoreContent(fmt.Sprintf("%s-%d", name, i), namespace), + ) + assert.NoErrorf( + err, + "expect no error from creating %s %s content %s", + name, namespace, err, + ) + } +} func validateContentList( assert *require.Assertions, clist []*model.ContentDoc, From cef3aa0e733bc99f33e8548f7e1978909eb6aeb2 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Wed, 14 Aug 2024 13:43:48 -0500 Subject: [PATCH 11/34] feat(arangodb_list_test.go): add custom content creation in tests for enhanced testing flexibility The addition of `createCustomTestContents` in the test suite allows for more flexible and specific testing scenarios. This function enables the creation of test contents with customizable parameters, improving the ability to test various edge cases and configurations. --- .../repository/arangodb/arangodb_list_test.go | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/internal/repository/arangodb/arangodb_list_test.go b/internal/repository/arangodb/arangodb_list_test.go index 1ca068a..1163402 100644 --- a/internal/repository/arangodb/arangodb_list_test.go +++ b/internal/repository/arangodb/arangodb_list_test.go @@ -19,6 +19,7 @@ func TestListContents(t *testing.T) { // Create test data createTestContents(assert, repo, 14) + createCustomTestContents(assert, repo, 14, "gallery", "genome") // Test initial list clist, err := repo.ListContents(0, 4, "") @@ -55,22 +56,6 @@ func TestListContents(t *testing.T) { ) } -func createTestContents( - assert *require.Assertions, - repo repository.ContentRepository, - count int, -) { - for i := 0; i < count; i++ { - time.Sleep(1 * time.Millisecond) - _, err := repo.AddContent( - testutils.NewStoreContent(fmt.Sprintf("gallery-%d", i), "genome"), - ) - assert.NoErrorf( - err, - "expect no error from creating gallery genome content %s", - err, - ) - } } func createCustomTestContents( From ed9c07da38e86e60c40fe89b20ada041aff9956f Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Wed, 14 Aug 2024 13:44:32 -0500 Subject: [PATCH 12/34] refactor(arangodb_list_test.go): parameterize namespace and name in tests for flexibility The test function `validateContentList` is updated to accept `name` and `namespace` as parameters, allowing it to be more flexible and reusable with different test scenarios. --- .../repository/arangodb/arangodb_list_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/repository/arangodb/arangodb_list_test.go b/internal/repository/arangodb/arangodb_list_test.go index 1163402..4e8addc 100644 --- a/internal/repository/arangodb/arangodb_list_test.go +++ b/internal/repository/arangodb/arangodb_list_test.go @@ -76,29 +76,29 @@ func createCustomTestContents( ) } } + func validateContentList( assert *require.Assertions, clist []*model.ContentDoc, + name, namespace string, ) { - nrxp := regexp.MustCompile(`gallery-\d+`) + nrxp := regexp.MustCompile(fmt.Sprintf(`%s-\d+`, name)) for idx, cnt := range clist { assert.Equal( cnt.Namespace, - "genome", - "should match the genome namespace", + namespace, + "should match the namespace", ) - assert.Regexp(nrxp, cnt.Name, "should match gallery names") + assert.Regexp(nrxp, cnt.Name, "should match names") assert.Equal( cnt.CreatedBy, "content@content.org", "should match the created_by", ) if idx != 0 { - assert.Truef( + assert.True( clist[idx-1].CreatedOn.After(clist[idx].CreatedOn), - "previous gallery record %s should be created after the current one %s", - clist[idx-1].CreatedOn.String(), - clist[idx].CreatedOn, + "previous record should be created after the current one", ) } } From b8ea6bed20c6dab492846f6bba661ef9d1005789 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Wed, 14 Aug 2024 17:50:17 -0500 Subject: [PATCH 13/34] feat(arangodb_list_test.go): add tests for content listing with filters This update introduces new test cases in `arangodb_list_test.go` to verify the behavior of the content listing functionality when filters are applied. The tests cover scenarios including simple filters, substring matching, and combination filters. Additionally, the tests check for proper error handling when no results are found due to stringent filter conditions. --- .../repository/arangodb/arangodb_list_test.go | 49 ++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/internal/repository/arangodb/arangodb_list_test.go b/internal/repository/arangodb/arangodb_list_test.go index 4e8addc..af1380e 100644 --- a/internal/repository/arangodb/arangodb_list_test.go +++ b/internal/repository/arangodb/arangodb_list_test.go @@ -18,14 +18,13 @@ func TestListContents(t *testing.T) { defer tearDown(repo) // Create test data - createTestContents(assert, repo, 14) createCustomTestContents(assert, repo, 14, "gallery", "genome") // Test initial list clist, err := repo.ListContents(0, 4, "") assert.NoError(err, "expect no error from listing content") assert.Len(clist, 5, "should have 5 content entries") - validateContentList(assert, clist) + validateContentList(assert, clist, "gallery", "genome") // Test pagination clist2, err := repo.ListContents( @@ -56,6 +55,52 @@ func TestListContents(t *testing.T) { ) } +func TestListContentsWithFilter(t *testing.T) { + t.Parallel() + assert, repo := setUp(t) + defer tearDown(repo) + + createCustomTestContents(assert, repo, 6, "sword", "longbottom") + createCustomTestContents(assert, repo, 8, "field", "drinkwater") + + // Test list with filter + filter := `FILTER cnt.namespace == "longbottom"` + clist, err := repo.ListContents(0, 14, filter) + assert.NoError(err, "expect no error from listing content with filter") + assert.Len(clist, 6, "should have 5 content entries") + validateContentList(assert, clist, "sword", "longbottom") + + // Test list with filter matching substring of slug + filter = `FILTER cnt.slug =~ "drink"` + clist, err = repo.ListContents(0, 15, filter) + assert.NoError( + err, + "expect no error from listing content with filter matching substring of slug", + ) + assert.Len(clist, 8, "should have 8 content entries") + validateContentList(assert, clist, "field", "drinkwater") + + // Test combination filter with multiple results + filter = `FILTER cnt.name =~ "sword-" AND cnt.namespace =~ "long"` + clist, err = repo.ListContents(0, 10, filter) + assert.NoError( + err, + "expect no error from listing content with combination filter (multiple results)", + ) + assert.Len(clist, 6, "should have 6 content entries") + validateContentList(assert, clist, "sword", "longbottom") + + // Test combination filter with no results + filter = `FILTER cnt.name == "sword-3" AND cnt.namespace =~ "drink"` + _, err = repo.ListContents(0, 10, filter) + assert.Error( + err, + "expect error from listing content with combination filter (no results)", + ) + assert.True( + repository.IsContentListNotFound(err), + "should be list not found type of error", + ) } func createCustomTestContents( From 7dd31c42af90b82249dcd29218ac94620e6d1277 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Wed, 14 Aug 2024 18:26:08 -0500 Subject: [PATCH 14/34] feat(arangodb_list_test.go): add comprehensive test cases for content filtering Enhanced the test suite for the ArangoDB repository by introducing a variety of test cases to validate content filtering functionality. These tests cover different scenarios including filtering by namespace, slug substring, and combinations of filters. This ensures that the filtering logic in the repository handles various conditions correctly and robustly, improving confidence in the application's data retrieval capabilities. --- .../repository/arangodb/arangodb_list_test.go | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/internal/repository/arangodb/arangodb_list_test.go b/internal/repository/arangodb/arangodb_list_test.go index af1380e..2d3bc49 100644 --- a/internal/repository/arangodb/arangodb_list_test.go +++ b/internal/repository/arangodb/arangodb_list_test.go @@ -63,6 +63,62 @@ func TestListContentsWithFilter(t *testing.T) { createCustomTestContents(assert, repo, 6, "sword", "longbottom") createCustomTestContents(assert, repo, 8, "field", "drinkwater") + testCases := []struct { + name string + filter string + expectedCount int + expectedName string + expectedNS string + expectError bool + }{ + { + name: "Filter by namespace", + filter: `FILTER cnt.namespace == "longbottom"`, + expectedCount: 6, + expectedName: "sword", + expectedNS: "longbottom", + expectError: false, + }, + { + name: "Filter by slug substring", + filter: `FILTER cnt.slug =~ "drink"`, + expectedCount: 8, + expectedName: "field", + expectedNS: "drinkwater", + expectError: false, + }, + { + name: "Combination filter with multiple results", + filter: `FILTER cnt.name =~ "sword-" AND cnt.namespace =~ "long"`, + expectedCount: 6, + expectedName: "sword", + expectedNS: "longbottom", + expectError: false, + }, + { + name: "Combination filter with no results", + filter: `FILTER cnt.name == "sword-3" AND cnt.namespace =~ "drink"`, + expectError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + clist, err := repo.ListContents(0, 15, tc.filter) + if tc.expectError { + assert.Error(err, "expect error from listing content") + assert.True( + repository.IsContentListNotFound(err), + "should be list not found type of error", + ) + } else { + assert.NoError(err, "expect no error from listing content") + assert.Len(clist, tc.expectedCount, "should have expected number of content entries") + validateContentList(assert, clist, tc.expectedName, tc.expectedNS) + } + }) + } +} // Test list with filter filter := `FILTER cnt.namespace == "longbottom"` clist, err := repo.ListContents(0, 14, filter) From ca56d822ed6f0677769929af9361b92896d613bb Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Wed, 14 Aug 2024 18:41:22 -0500 Subject: [PATCH 15/34] refactor(arangodb_list_test.go): modularize test cases for content listing with filters This change introduces a structured approach to testing content listing with filters in the ArangoDB repository. By defining a `TestCaseWithFilterandCursor` struct, we encapsulate all parameters and expected results for each test scenario. This makes the test cases easier to read and maintain. Additionally, the use of a helper function `createTestCases` to generate a slice of test cases, and another function `runListContentsWithFilterAndCursorSubtest` to execute these tests, further organizes the code and enhances reusability. --- .../repository/arangodb/arangodb_list_test.go | 120 +++++++++++++----- 1 file changed, 91 insertions(+), 29 deletions(-) diff --git a/internal/repository/arangodb/arangodb_list_test.go b/internal/repository/arangodb/arangodb_list_test.go index 2d3bc49..3284b51 100644 --- a/internal/repository/arangodb/arangodb_list_test.go +++ b/internal/repository/arangodb/arangodb_list_test.go @@ -12,6 +12,17 @@ import ( "github.com/stretchr/testify/require" ) +type TestCaseWithFilterandCursor struct { + name string + filter string + initialLimit int64 + subsequentLimit int64 + expectedName string + expectedNS string + expectedInitialCount int + expectedSubsequentCount int +} + func TestListContents(t *testing.T) { t.Parallel() assert, repo := setUp(t) @@ -119,44 +130,95 @@ func TestListContentsWithFilter(t *testing.T) { }) } } - // Test list with filter - filter := `FILTER cnt.namespace == "longbottom"` - clist, err := repo.ListContents(0, 14, filter) - assert.NoError(err, "expect no error from listing content with filter") - assert.Len(clist, 6, "should have 5 content entries") - validateContentList(assert, clist, "sword", "longbottom") - - // Test list with filter matching substring of slug - filter = `FILTER cnt.slug =~ "drink"` - clist, err = repo.ListContents(0, 15, filter) + +func createTestCases() []TestCaseWithFilterandCursor { + return []TestCaseWithFilterandCursor{ + { + name: "Filter by namespace", + filter: `FILTER cnt.namespace == "hogwarts"`, + initialLimit: 5, + subsequentLimit: 5, + expectedName: "wand", + expectedNS: "hogwarts", + expectedInitialCount: 6, + expectedSubsequentCount: 5, + }, + { + name: "Filter by slug substring", + filter: `FILTER cnt.slug =~ "potion"`, + initialLimit: 7, + subsequentLimit: 7, + expectedName: "potion", + expectedNS: "hogsmeade", + expectedInitialCount: 8, + expectedSubsequentCount: 5, + }, + { + name: "Combination filter", + filter: `FILTER cnt.name =~ "wand-" AND cnt.namespace =~ "hog"`, + initialLimit: 6, + subsequentLimit: 6, + expectedName: "wand", + expectedNS: "hogwarts", + expectedInitialCount: 7, + expectedSubsequentCount: 4, + }, + } +} + +func TestListContentsWithFilterAndCursor(t *testing.T) { + t.Parallel() + assert, repo := setUp(t) + defer tearDown(repo) + createCustomTestContents(assert, repo, 10, "wand", "hogwarts") + createCustomTestContents(assert, repo, 12, "potion", "hogsmeade") + + for _, tc := range createTestCases() { + t.Run(tc.name, func(t *testing.T) { + runListContentsWithFilterAndCursorSubtest(assert, repo, tc) + }) + } +} + +func runListContentsWithFilterAndCursorSubtest( + assert *require.Assertions, + repo repository.ContentRepository, + tc TestCaseWithFilterandCursor, +) { + // Initial list + clist, err := repo.ListContents(0, tc.initialLimit, tc.filter) assert.NoError( err, - "expect no error from listing content with filter matching substring of slug", + "expect no error from listing content with filter", ) - assert.Len(clist, 8, "should have 8 content entries") - validateContentList(assert, clist, "field", "drinkwater") + assert.Len( + clist, + tc.expectedInitialCount, + "should have expected number of content entries", + ) + validateContentList(assert, clist, tc.expectedName, tc.expectedNS) - // Test combination filter with multiple results - filter = `FILTER cnt.name =~ "sword-" AND cnt.namespace =~ "long"` - clist, err = repo.ListContents(0, 10, filter) + // Subsequent list with cursor + clist2, err := repo.ListContents( + clist[len(clist)-1].CreatedOn.UnixMilli(), + tc.subsequentLimit, + tc.filter, + ) assert.NoError( err, - "expect no error from listing content with combination filter (multiple results)", + "expect no error from listing content with filter and cursor", ) - assert.Len(clist, 6, "should have 6 content entries") - validateContentList(assert, clist, "sword", "longbottom") - - // Test combination filter with no results - filter = `FILTER cnt.name == "sword-3" AND cnt.namespace =~ "drink"` - _, err = repo.ListContents(0, 10, filter) - assert.Error( - err, - "expect error from listing content with combination filter (no results)", + assert.Len( + clist2, + tc.expectedSubsequentCount, + "should have expected number of content entries", ) - assert.True( - repository.IsContentListNotFound(err), - "should be list not found type of error", + assert.Exactly( + clist[len(clist)-1], + clist2[0], + "should be identical content object", ) + validateContentList(assert, clist2, tc.expectedName, tc.expectedNS) } func createCustomTestContents( From 79ba2b88fd2c653b66386594f6ac136b814143e3 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 10:17:14 -0500 Subject: [PATCH 16/34] refactor(arangodb_list_test.go): centralize test case struct and generator This change centralizes the `TestCaseWithFilterandCursor` struct and its associated `createTestCases` function into the `testutils` package. This refactoring improves code reusability and maintainability by allowing other tests across the project to utilize the same test case structure and generator function without duplicating code. Additionally, it standardizes the naming convention of struct fields to follow Go's convention of capitalizing public fields, enhancing consistency across the codebase. --- .../repository/arangodb/arangodb_list_test.go | 66 +++---------------- internal/testutils/testutils.go | 46 +++++++++++++ 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/internal/repository/arangodb/arangodb_list_test.go b/internal/repository/arangodb/arangodb_list_test.go index 3284b51..37848be 100644 --- a/internal/repository/arangodb/arangodb_list_test.go +++ b/internal/repository/arangodb/arangodb_list_test.go @@ -12,17 +12,6 @@ import ( "github.com/stretchr/testify/require" ) -type TestCaseWithFilterandCursor struct { - name string - filter string - initialLimit int64 - subsequentLimit int64 - expectedName string - expectedNS string - expectedInitialCount int - expectedSubsequentCount int -} - func TestListContents(t *testing.T) { t.Parallel() assert, repo := setUp(t) @@ -131,41 +120,6 @@ func TestListContentsWithFilter(t *testing.T) { } } -func createTestCases() []TestCaseWithFilterandCursor { - return []TestCaseWithFilterandCursor{ - { - name: "Filter by namespace", - filter: `FILTER cnt.namespace == "hogwarts"`, - initialLimit: 5, - subsequentLimit: 5, - expectedName: "wand", - expectedNS: "hogwarts", - expectedInitialCount: 6, - expectedSubsequentCount: 5, - }, - { - name: "Filter by slug substring", - filter: `FILTER cnt.slug =~ "potion"`, - initialLimit: 7, - subsequentLimit: 7, - expectedName: "potion", - expectedNS: "hogsmeade", - expectedInitialCount: 8, - expectedSubsequentCount: 5, - }, - { - name: "Combination filter", - filter: `FILTER cnt.name =~ "wand-" AND cnt.namespace =~ "hog"`, - initialLimit: 6, - subsequentLimit: 6, - expectedName: "wand", - expectedNS: "hogwarts", - expectedInitialCount: 7, - expectedSubsequentCount: 4, - }, - } -} - func TestListContentsWithFilterAndCursor(t *testing.T) { t.Parallel() assert, repo := setUp(t) @@ -173,8 +127,8 @@ func TestListContentsWithFilterAndCursor(t *testing.T) { createCustomTestContents(assert, repo, 10, "wand", "hogwarts") createCustomTestContents(assert, repo, 12, "potion", "hogsmeade") - for _, tc := range createTestCases() { - t.Run(tc.name, func(t *testing.T) { + for _, tc := range testutils.CreateTestCases() { + t.Run(tc.Name, func(t *testing.T) { runListContentsWithFilterAndCursorSubtest(assert, repo, tc) }) } @@ -183,26 +137,26 @@ func TestListContentsWithFilterAndCursor(t *testing.T) { func runListContentsWithFilterAndCursorSubtest( assert *require.Assertions, repo repository.ContentRepository, - tc TestCaseWithFilterandCursor, + tc testutils.TestCaseWithFilterandCursor, ) { // Initial list - clist, err := repo.ListContents(0, tc.initialLimit, tc.filter) + clist, err := repo.ListContents(0, tc.InitialLimit, tc.Filter) assert.NoError( err, "expect no error from listing content with filter", ) assert.Len( clist, - tc.expectedInitialCount, + tc.ExpectedInitialCount, "should have expected number of content entries", ) - validateContentList(assert, clist, tc.expectedName, tc.expectedNS) + validateContentList(assert, clist, tc.ExpectedName, tc.ExpectedNS) // Subsequent list with cursor clist2, err := repo.ListContents( clist[len(clist)-1].CreatedOn.UnixMilli(), - tc.subsequentLimit, - tc.filter, + tc.SubsequentLimit, + tc.Filter, ) assert.NoError( err, @@ -210,7 +164,7 @@ func runListContentsWithFilterAndCursorSubtest( ) assert.Len( clist2, - tc.expectedSubsequentCount, + tc.ExpectedSubsequentCount, "should have expected number of content entries", ) assert.Exactly( @@ -218,7 +172,7 @@ func runListContentsWithFilterAndCursorSubtest( clist2[0], "should be identical content object", ) - validateContentList(assert, clist2, tc.expectedName, tc.expectedNS) + validateContentList(assert, clist2, tc.ExpectedName, tc.ExpectedNS) } func createCustomTestContents( diff --git a/internal/testutils/testutils.go b/internal/testutils/testutils.go index 22956ff..7e2a1a3 100644 --- a/internal/testutils/testutils.go +++ b/internal/testutils/testutils.go @@ -8,6 +8,17 @@ import ( "github.com/dictyBase/modware-content/internal/model" ) +type TestCaseWithFilterandCursor struct { + Name string + Filter string + InitialLimit int64 + SubsequentLimit int64 + ExpectedName string + ExpectedNS string + ExpectedInitialCount int + ExpectedSubsequentCount int +} + type ContentJSON struct { Paragraph string `json:"paragraph"` Text string `json:"text"` @@ -37,3 +48,38 @@ func ContentFromStore(jsctnt string) (*ContentJSON, error) { return ctnt, nil } + +func CreateTestCases() []TestCaseWithFilterandCursor { + return []TestCaseWithFilterandCursor{ + { + Name: "Filter by namespace", + Filter: `FILTER cnt.namespace == "hogwarts"`, + InitialLimit: 5, + SubsequentLimit: 5, + ExpectedName: "wand", + ExpectedNS: "hogwarts", + ExpectedInitialCount: 6, + ExpectedSubsequentCount: 5, + }, + { + Name: "Filter by slug substring", + Filter: `FILTER cnt.slug =~ "potion"`, + InitialLimit: 7, + SubsequentLimit: 7, + ExpectedName: "potion", + ExpectedNS: "hogsmeade", + ExpectedInitialCount: 8, + ExpectedSubsequentCount: 5, + }, + { + Name: "Combination filter", + Filter: `FILTER cnt.name =~ "wand-" AND cnt.namespace =~ "hog"`, + InitialLimit: 6, + SubsequentLimit: 6, + ExpectedName: "wand", + ExpectedNS: "hogwarts", + ExpectedInitialCount: 7, + ExpectedSubsequentCount: 4, + }, + } +} From f7ea20d89391860a4d1f9d8d58175324a4289ef1 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 14:37:46 -0500 Subject: [PATCH 17/34] feat(go.mod): update go-genproto to latest version and add new dependencies The go-genproto package is updated to a newer version to incorporate the latest changes and improvements from the upstream repository. Additionally, new dependencies, `github.com/emirpasic/gods` and `github.com/jinzhu/now`, are added to enhance data structure handling and provide better date-time utilities, respectively, which are essential for the ongoing development and feature enhancements. --- go.mod | 4 +++- go.sum | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 98d2be5..e24a62b 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ require ( github.com/arangodb/go-driver v1.6.2 github.com/dictyBase/aphgrpc v1.4.2 github.com/dictyBase/arangomanager v0.4.0 - github.com/dictyBase/go-genproto v0.0.0-20231030202356-522cb6f9976a + github.com/dictyBase/go-genproto v0.0.0-20240815171842-0da89d7f57e4 github.com/go-playground/validator/v10 v10.22.0 github.com/golang/protobuf v1.5.4 github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 @@ -19,11 +19,13 @@ require ( github.com/arangodb/go-velocypack v0.0.0-20200318135517-5af53c29c67e // indirect github.com/cpuguy83/go-md2man/v2 v2.0.4 // indirect github.com/davecgh/go-spew v1.1.1 // indirect + github.com/emirpasic/gods v1.18.1 // indirect github.com/fatih/structs v1.1.0 // indirect github.com/gabriel-vasile/mimetype v1.4.3 // indirect github.com/go-playground/locales v0.14.1 // indirect github.com/go-playground/universal-translator v0.18.1 // indirect github.com/gogo/protobuf v1.3.2 // indirect + github.com/jinzhu/now v1.1.5 // indirect github.com/klauspost/compress v1.17.2 // indirect github.com/leodido/go-urn v1.4.0 // indirect github.com/mwitkow/go-proto-validators v0.2.0 // indirect diff --git a/go.sum b/go.sum index fea7929..b2932d3 100644 --- a/go.sum +++ b/go.sum @@ -26,8 +26,9 @@ github.com/dictyBase/aphgrpc v1.4.2 h1:jkNyIpUBATNVNTM99TfAHln8UEpzyvNNNuM2+UY6t github.com/dictyBase/aphgrpc v1.4.2/go.mod h1:rzi2U95QntAuS6XpqespgZmQ8Ujo2Sq2R5FyhW5adA8= github.com/dictyBase/arangomanager v0.4.0 h1:+HgmadUY0qXFMLfZocktynbERkX/FgWzC3vd/MMebnU= github.com/dictyBase/arangomanager v0.4.0/go.mod h1:SmElsQJN3EbTjfYCkjS7/uyUkNVwaO2qJB9Mqrs0ND8= -github.com/dictyBase/go-genproto v0.0.0-20231030202356-522cb6f9976a h1:V0xN+gehQLCJrqFPI1nNd8wCIi6Z0ZQgmoZ6dmkPlV0= -github.com/dictyBase/go-genproto v0.0.0-20231030202356-522cb6f9976a/go.mod h1:KY6iUqXl1lLq81WTW9u5DfZE58xM61PsGLSLwwtePlk= +github.com/dictyBase/go-genproto v0.0.0-20240815171842-0da89d7f57e4 h1:kxY0rlODJu8qk5jXv9dzeOpOl3hV8r66y1YByPKCDIs= +github.com/dictyBase/go-genproto v0.0.0-20240815171842-0da89d7f57e4/go.mod h1:KY6iUqXl1lLq81WTW9u5DfZE58xM61PsGLSLwwtePlk= +github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc= github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= @@ -84,6 +85,7 @@ github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+ github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 h1:UH//fgunKIs4JdUbpDl1VZCDaL56wXCB/5+wF6uHfaI= github.com/grpc-ecosystem/go-grpc-middleware v1.4.0/go.mod h1:g5qyo/la0ALbONm6Vbp88Yd8NsDy6rZz+RcrMPxvld8= github.com/grpc-ecosystem/grpc-gateway v1.11.3/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= +github.com/jinzhu/now v1.1.5 h1:/o9tlHleP7gOFmsnYNz3RGnqzefHA47wQpKrrdTIwXQ= github.com/jinzhu/now v1.1.5/go.mod h1:d3SSVoowX0Lcu0IBviAWJpolVfI5UJVZZ7cO71lE/z8= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= From 45e4217860b3e94aa112521158290cd442bdf7b7 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 14:38:02 -0500 Subject: [PATCH 18/34] feat(arangodb/field.go): add FilterMap function to map filter attributes to DB fields Introduces a new function `FilterMap` in the ArangoDB repository layer to provide a clear mapping between filter attributes used in the application and their corresponding database fields. --- internal/repository/arangodb/field.go | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 internal/repository/arangodb/field.go diff --git a/internal/repository/arangodb/field.go b/internal/repository/arangodb/field.go new file mode 100644 index 0000000..c3005dc --- /dev/null +++ b/internal/repository/arangodb/field.go @@ -0,0 +1,11 @@ +package arangodb + +// FilterMap provides mapping of filter attributes to database fields. +func FilterMap() map[string]string { + return map[string]string{ + "name": "cnt.name", + "namespace": "cnt.namespace", + "slug": "cnt.slug", + "created_by": "cnt.created_by", + } +} From 180f40d11d5956a95719ba67cc8e30624f9bec76 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 14:39:33 -0500 Subject: [PATCH 19/34] feat(service.go): add query parsing and AQL statement generation for filters The addition of the `filterStrToQuery` function in `service.go` allows the application to convert filter strings into AQL (ArangoDB Query Language) statements. This enhancement supports dynamic querying based on user-provided filters, improving the flexibility and usability of the data retrieval process in applications using ArangoDB. The function leverages the `query` and `arangodb` packages to parse the filter string and generate the corresponding AQL statement, handling errors appropriately. --- internal/app/service/service.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/app/service/service.go b/internal/app/service/service.go index 33540c6..4a8c3bd 100644 --- a/internal/app/service/service.go +++ b/internal/app/service/service.go @@ -6,11 +6,13 @@ import ( "strconv" "github.com/dictyBase/aphgrpc" + "github.com/dictyBase/arangomanager/query" "github.com/dictyBase/go-genproto/dictybaseapis/api/jsonapi" "github.com/dictyBase/go-genproto/dictybaseapis/content" "github.com/dictyBase/modware-content/internal/message" "github.com/dictyBase/modware-content/internal/model" "github.com/dictyBase/modware-content/internal/repository" + "github.com/dictyBase/modware-content/internal/repository/arangodb" "github.com/go-playground/validator/v10" "github.com/golang/protobuf/ptypes/empty" ) @@ -187,3 +189,20 @@ func (srv *ContentService) DeleteContent( return &empty.Empty{}, nil } + +func filterStrToQuery(filter string) (string, error) { + var empty string + if len(filter) == 0 { + return empty, nil + } + p, err := query.ParseFilterString(filter) + if err != nil { + return empty, fmt.Errorf("error in parsing filter string") + } + q, err := query.GenQualifiedAQLFilterStatement(arangodb.FilterMap(), p) + if err != nil { + return empty, fmt.Errorf("error in generating aql statement") + } + + return q, nil +} From eec73862a29ec7ba913fb62240bd94e891b5ddc4 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 14:39:57 -0500 Subject: [PATCH 20/34] feat(service.go): add ListContents method to ContentService for content retrieval based on filters The new `ListContents` method in `ContentService` allows fetching a list of content entries based on provided filters and pagination parameters. This method supports dynamic query generation based on the `ListParameters` input, which includes a limit and a filter string. --- internal/app/service/service.go | 53 ++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/internal/app/service/service.go b/internal/app/service/service.go index 4a8c3bd..c9e2ec1 100644 --- a/internal/app/service/service.go +++ b/internal/app/service/service.go @@ -132,7 +132,8 @@ func (srv *ContentService) buildContent( UpdatedAt: aphgrpc.TimestampProto(mcont.UpdatedOn), Content: mcont.Content, }, - }} + }, + } } func (srv *ContentService) StoreContent( @@ -190,6 +191,56 @@ func (srv *ContentService) DeleteContent( return &empty.Empty{}, nil } +func (srv *ContentService) ListContents( + ctx context.Context, + req *content.ListParameters, +) (*content.ContentCollection, error) { + limit := int64(10) + if req.Limit > 0 { + limit = req.Limit + } + astmt, err := filterStrToQuery(req.Filter) + if err != nil { + return nil, aphgrpc.HandleInvalidParamError(ctx, err) + } + cntModel, err := srv.repo.ListContents(req.Cursor, limit, astmt) + if err != nil { + if repository.IsContentListNotFound(err) { + return nil, aphgrpc.HandleNotFoundError(ctx, err) + } + } + cntDataSlice := make([]*content.ContentCollection_Data, 0) + for _, cntD := range cntModel { + cntDataSlice = append(cntDataSlice, &content.ContentCollection_Data{ + Id: cntD.Key, + Attributes: &content.ContentAttributes{ + Name: cntD.Name, + Namespace: cntD.Namespace, + Slug: cntD.Slug, + CreatedBy: cntD.CreatedBy, + UpdatedBy: cntD.UpdatedBy, + Content: cntD.Content, + CreatedAt: aphgrpc.TimestampProto(cntD.CreatedOn), + UpdatedAt: aphgrpc.TimestampProto(cntD.UpdatedOn), + }, + }) + } + + cnt := &content.ContentCollection{} + if len(cntDataSlice) < int(limit)-2 { // fewer result than limit + cnt.Data = cntDataSlice + cnt.Meta = &content.Meta{Limit: req.Limit} + return cnt, nil + } + cnt.Data = cntDataSlice[:len(cntDataSlice)-1] + cnt.Meta = &content.Meta{ + Limit: limit, + NextCursor: cntModel[len(cntModel)-1].CreatedOn.UnixMilli(), + } + + return cnt, nil +} + func filterStrToQuery(filter string) (string, error) { var empty string if len(filter) == 0 { From cd8fcb257798d332ec8b2306a5bca333e32ae83e Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 17:55:13 -0500 Subject: [PATCH 21/34] feat(service_test.go): add validateListContents function to enhance testing capabilities The new function `validateListContents` is introduced to streamline the validation of content list properties in tests. It uses a structured approach by accepting parameters in a struct, allowing for more organized and readable tests. --- internal/app/service/service_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/internal/app/service/service_test.go b/internal/app/service/service_test.go index 8c2d924..4c24761 100644 --- a/internal/app/service/service_test.go +++ b/internal/app/service/service_test.go @@ -283,6 +283,28 @@ func TestDeleteContent(t *testing.T) { assert.NoError(err, "expect no error from deleting content") } +type validateListContentsParams struct { + Assert *require.Assertions + Data []*content.ContentCollection_Data + NameRegex *regexp.Regexp + Namespace string +} + +func validateListContents(params validateListContentsParams) { + for _, cnt := range params.Data { + params.Assert.Regexp( + params.NameRegex, + cnt.Attributes.Name, + "name should match", + ) + params.Assert.Equal( + cnt.Attributes.Namespace, + params.Namespace, + "namespace should match", + ) + } +} + func testContentProperties( assert *require.Assertions, sct, nct *content.Content, From 53a0e9fc1aba2acfc00a987701117f76261e2ba3 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 17:55:37 -0500 Subject: [PATCH 22/34] feat(service_test.go): add storeMultipleContents function to handle bulk content storage in tests The new function `storeMultipleContents` allows for bulk storage of content items in tests, improving the efficiency of testing scenarios that require multiple content items to be stored. --- internal/app/service/service_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/internal/app/service/service_test.go b/internal/app/service/service_test.go index 4c24761..06efe9d 100644 --- a/internal/app/service/service_test.go +++ b/internal/app/service/service_test.go @@ -305,6 +305,28 @@ func validateListContents(params validateListContentsParams) { } } +func storeMultipleContents( + client content.ContentServiceClient, + assert *require.Assertions, + count int, + name, namespace string, +) { + for i := 0; i < count; i++ { + _, err := client.StoreContent( + context.Background(), + &content.StoreContentRequest{ + Data: &content.StoreContentRequest_Data{ + Attributes: testutils.NewStoreContent( + fmt.Sprintf("%s-%d", name, i), + namespace, + ), + }, + }, + ) + assert.NoError(err, "expect no error from storing content") + } +} + func testContentProperties( assert *require.Assertions, sct, nct *content.Content, From 90b2afb730c170694edc3ae6c984af629008279b Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 17:55:55 -0500 Subject: [PATCH 23/34] feat(service_test.go): add TestListContentsService to validate content listing functionality The addition of `TestListContentsService` in `service_test.go` ensures that the service can handle listing contents correctly. This test checks if the service can return a limited number of content items and validates the content names against a specific pattern. --- internal/app/service/service_test.go | 33 +++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/internal/app/service/service_test.go b/internal/app/service/service_test.go index 06efe9d..67df2bf 100644 --- a/internal/app/service/service_test.go +++ b/internal/app/service/service_test.go @@ -3,8 +3,10 @@ package service import ( "context" "encoding/json" + "fmt" "net" "os" + "regexp" "testing" "time" @@ -24,6 +26,13 @@ import ( "google.golang.org/grpc/test/bufconn" ) +type validateListContentsParams struct { + Assert *require.Assertions + Data []*content.ContentCollection_Data + NameRegex *regexp.Regexp + Namespace string +} + type MockMessage struct{} func (msn *MockMessage) Publish(subject string, cont *content.Content) error { @@ -283,11 +292,25 @@ func TestDeleteContent(t *testing.T) { assert.NoError(err, "expect no error from deleting content") } -type validateListContentsParams struct { - Assert *require.Assertions - Data []*content.ContentCollection_Data - NameRegex *regexp.Regexp - Namespace string +func TestListContentsService(t *testing.T) { + t.Parallel() + client, assert := setup(t) + + name, namespace := "catalog", "dsc" + storeMultipleContents(client, assert, 10, name, namespace) + resp, err := client.ListContents( + context.Background(), + &content.ListParameters{Limit: 5}, + ) + assert.NoError(err, "expect no error from listing contents") + assert.Len(resp.Data, 5, "should return 5 contents") + nameRegex := regexp.MustCompile(fmt.Sprintf(`%s-\d+`, name)) + validateListContents(validateListContentsParams{ + Assert: assert, + Data: resp.Data, + NameRegex: nameRegex, + Namespace: namespace, + }) } func validateListContents(params validateListContentsParams) { From 879f3a510a915eb3e1072a0ee0a8f4328031f6e0 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 18:00:12 -0500 Subject: [PATCH 24/34] refactor(service_test.go): encapsulate parameters of storeMultipleContents into a struct This change improves the readability and maintainability of the `storeMultipleContents` function by encapsulating its parameters into a single struct. This approach reduces the number of arguments passed to the function, making the code cleaner and easier to manage, especially when dealing with multiple parameters. --- internal/app/service/service_test.go | 33 ++++++++++++++++++---------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/internal/app/service/service_test.go b/internal/app/service/service_test.go index 67df2bf..91dcb63 100644 --- a/internal/app/service/service_test.go +++ b/internal/app/service/service_test.go @@ -297,7 +297,13 @@ func TestListContentsService(t *testing.T) { client, assert := setup(t) name, namespace := "catalog", "dsc" - storeMultipleContents(client, assert, 10, name, namespace) + storeMultipleContents(storeMultipleContentsParams{ + Client: client, + Assert: assert, + Count: 10, + Name: name, + Namespace: namespace, + }) resp, err := client.ListContents( context.Background(), &content.ListParameters{Limit: 5}, @@ -328,25 +334,28 @@ func validateListContents(params validateListContentsParams) { } } -func storeMultipleContents( - client content.ContentServiceClient, - assert *require.Assertions, - count int, - name, namespace string, -) { - for i := 0; i < count; i++ { - _, err := client.StoreContent( +type storeMultipleContentsParams struct { + Client content.ContentServiceClient + Assert *require.Assertions + Count int + Name string + Namespace string +} + +func storeMultipleContents(params storeMultipleContentsParams) { + for i := 0; i < params.Count; i++ { + _, err := params.Client.StoreContent( context.Background(), &content.StoreContentRequest{ Data: &content.StoreContentRequest_Data{ Attributes: testutils.NewStoreContent( - fmt.Sprintf("%s-%d", name, i), - namespace, + fmt.Sprintf("%s-%d", params.Name, i), + params.Namespace, ), }, }, ) - assert.NoError(err, "expect no error from storing content") + params.Assert.NoError(err, "expect no error from storing content") } } From 13e9000bccf15a0cbacd00962656c117f360b974 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 19:26:18 -0500 Subject: [PATCH 25/34] fix(service.go): correct pagination logic in ListContents function The pagination logic in the ListContents function of the ContentService has been corrected to properly handle cases where the number of items fetched is exactly one more than the requested limit. This change ensures that the last item is used to populate the NextCursor for subsequent requests, rather than being included in the current result set. This adjustment aligns with common pagination practices, where the presence of an additional item indicates more data is available. --- internal/app/service/service.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/internal/app/service/service.go b/internal/app/service/service.go index c9e2ec1..c5cf2f7 100644 --- a/internal/app/service/service.go +++ b/internal/app/service/service.go @@ -227,17 +227,16 @@ func (srv *ContentService) ListContents( } cnt := &content.ContentCollection{} - if len(cntDataSlice) < int(limit)-2 { // fewer result than limit - cnt.Data = cntDataSlice - cnt.Meta = &content.Meta{Limit: req.Limit} + if len(cntDataSlice) == int(limit)+1 { + cnt.Data = cntDataSlice[:len(cntDataSlice)-1] + cnt.Meta = &content.Meta{ + Limit: limit, + NextCursor: cntModel[len(cntModel)-1].CreatedOn.UnixMilli(), + } return cnt, nil } - cnt.Data = cntDataSlice[:len(cntDataSlice)-1] - cnt.Meta = &content.Meta{ - Limit: limit, - NextCursor: cntModel[len(cntModel)-1].CreatedOn.UnixMilli(), - } - + cnt.Data = cntDataSlice + cnt.Meta = &content.Meta{Limit: req.Limit} return cnt, nil } From c92cfbb13e57cb3d09eb40f82fab1ce2b18c2b57 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 19:26:50 -0500 Subject: [PATCH 26/34] feat(service_test.go): extend TestListContentsService to validate pagination The test now includes additional checks to ensure that pagination through the content list works correctly. It tests fetching subsequent pages using the next cursor and validates that the correct number of records is returned for each request. This ensures that the pagination logic in the ListContents service is functioning as expected. --- internal/app/service/service_test.go | 33 ++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/internal/app/service/service_test.go b/internal/app/service/service_test.go index 91dcb63..394f02e 100644 --- a/internal/app/service/service_test.go +++ b/internal/app/service/service_test.go @@ -317,6 +317,39 @@ func TestListContentsService(t *testing.T) { NameRegex: nameRegex, Namespace: namespace, }) + assert.Greater( + resp.Meta.NextCursor, + int64(0), + "should have more record to fetch", + ) + resp2, err := client.ListContents( + context.Background(), + &content.ListParameters{Limit: 7, Cursor: resp.Meta.NextCursor}, + ) + assert.NoError(err, "expect no error from listing contents") + assert.Len(resp2.Data, 5, "should return 5 contents") + validateListContents(validateListContentsParams{ + Assert: assert, + Data: resp2.Data, + NameRegex: nameRegex, + Namespace: namespace, + }) + assert.Equal( + resp2.Meta.NextCursor, + int64(0), + "should not have any more record to fetch", + ) + resp2, err = client.ListContents( + context.Background(), + &content.ListParameters{Limit: 10}, + ) + assert.NoError(err, "expect no error from listing contents") + assert.Len(resp2.Data, 10, "should return 10 contents") + assert.Equal( + resp2.Meta.NextCursor, + int64(0), + "should not have any more record to fetch", + ) } func validateListContents(params validateListContentsParams) { From c033cac71528c6753a068a70f450b3d1f53e316a Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 20:30:43 -0500 Subject: [PATCH 27/34] fix(service.go): enhance error messages to include underlying errors By wrapping the original errors in the error messages, this change provides more detailed context when errors occur during the parsing of filter strings and the generation of AQL statements. This will aid in debugging by exposing the root causes of failures more clearly. --- internal/app/service/service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/app/service/service.go b/internal/app/service/service.go index c5cf2f7..450c3e5 100644 --- a/internal/app/service/service.go +++ b/internal/app/service/service.go @@ -247,11 +247,11 @@ func filterStrToQuery(filter string) (string, error) { } p, err := query.ParseFilterString(filter) if err != nil { - return empty, fmt.Errorf("error in parsing filter string") + return empty, fmt.Errorf("error in parsing filter string %w", err) } q, err := query.GenQualifiedAQLFilterStatement(arangodb.FilterMap(), p) if err != nil { - return empty, fmt.Errorf("error in generating aql statement") + return empty, fmt.Errorf("error in generating aql statement %w", err) } return q, nil From c46d4fda5a0161d2f171ddcf2ec043994ab8934f Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Thu, 15 Aug 2024 20:31:19 -0500 Subject: [PATCH 28/34] refactor(service.go): remove unused jsonapi import and Healthz method The jsonapi import is removed because it is no longer used in the service.go file, which cleans up the dependencies and reduces the complexity of the import section. Additionally, the Healthz method is removed as it appears to be unused, simplifying the ContentService structure and focusing on the essential functionalities. --- internal/app/service/service.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/internal/app/service/service.go b/internal/app/service/service.go index 450c3e5..e900a64 100644 --- a/internal/app/service/service.go +++ b/internal/app/service/service.go @@ -7,7 +7,6 @@ import ( "github.com/dictyBase/aphgrpc" "github.com/dictyBase/arangomanager/query" - "github.com/dictyBase/go-genproto/dictybaseapis/api/jsonapi" "github.com/dictyBase/go-genproto/dictybaseapis/content" "github.com/dictyBase/modware-content/internal/message" "github.com/dictyBase/modware-content/internal/model" @@ -61,13 +60,6 @@ func NewContentService(srvP *Params) (*ContentService, error) { }, nil } -func (srv *ContentService) Healthz( - ctx context.Context, - rdr *jsonapi.HealthzIdRequest, -) (*empty.Empty, error) { - return &empty.Empty{}, nil -} - func (srv *ContentService) GetContentBySlug( ctx context.Context, rdr *content.ContentRequest, From e14b21f8db721e54254b666dd924bbb2133024ec Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Fri, 16 Aug 2024 10:28:39 -0500 Subject: [PATCH 29/34] refactor(service.go): improve variable naming and error messages in filterStrToQuery Variable names in the filterStrToQuery function are updated to be more descriptive, enhancing code readability. The error messages are also enhanced to include the filter string that caused the error, making debugging more straightforward by providing more context when errors occur. --- internal/app/service/service.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/internal/app/service/service.go b/internal/app/service/service.go index e900a64..9d614b9 100644 --- a/internal/app/service/service.go +++ b/internal/app/service/service.go @@ -237,14 +237,25 @@ func filterStrToQuery(filter string) (string, error) { if len(filter) == 0 { return empty, nil } - p, err := query.ParseFilterString(filter) + parsedStr, err := query.ParseFilterString(filter) if err != nil { - return empty, fmt.Errorf("error in parsing filter string %w", err) + return empty, fmt.Errorf( + "error in parsing filter string '%s': %w", + filter, + err, + ) } - q, err := query.GenQualifiedAQLFilterStatement(arangodb.FilterMap(), p) + queryStr, err := query.GenQualifiedAQLFilterStatement( + arangodb.FilterMap(), + parsedStr, + ) if err != nil { - return empty, fmt.Errorf("error in generating aql statement %w", err) + return empty, fmt.Errorf( + "error in generating AQL statement for filter '%s': %w", + filter, + err, + ) } - return q, nil + return queryStr, nil } From af022e05df944b76c605b232682ac3817f9c802f Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Fri, 16 Aug 2024 10:32:06 -0500 Subject: [PATCH 30/34] fix(service.go): handle generic errors in ListContents function Adds error handling for generic errors in the ListContents function of the ContentService. This ensures that all types of errors are appropriately managed and responded to, improving the robustness and reliability of the error handling mechanism in the service layer. --- internal/app/service/service.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/app/service/service.go b/internal/app/service/service.go index 9d614b9..45c33b7 100644 --- a/internal/app/service/service.go +++ b/internal/app/service/service.go @@ -200,6 +200,7 @@ func (srv *ContentService) ListContents( if repository.IsContentListNotFound(err) { return nil, aphgrpc.HandleNotFoundError(ctx, err) } + return nil, aphgrpc.HandleGetError(ctx, err) } cntDataSlice := make([]*content.ContentCollection_Data, 0) for _, cntD := range cntModel { From 81da92a6e94cd4e103a96fd4587ffd5b4bc61db2 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Fri, 16 Aug 2024 11:22:56 -0500 Subject: [PATCH 31/34] chore(.gitignore): add .aider* to ignore list to prevent tracking of aide files Adding .aider* to the .gitignore file ensures that any files with the .aider extension, which are typically used for temporary or auxiliary purposes, are not tracked by Git. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index a67498b..1af7659 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ vendor/ .plandex +.aider* From 6727c32f0e0ed47d65ad32b5a4294c757632acc1 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Fri, 16 Aug 2024 11:23:13 -0500 Subject: [PATCH 32/34] feat(service_test.go): add tests for content filtering in ListContentsService Introduces new test cases in `service_test.go` to verify the functionality of filtering contents by different attributes such as namespace and name. This ensures that the ListContentsService can accurately filter contents based on specified criteria, enhancing the robustness and usability of the service in environments where content differentiation is crucial. --- internal/app/service/service_test.go | 68 ++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/internal/app/service/service_test.go b/internal/app/service/service_test.go index 394f02e..62130a5 100644 --- a/internal/app/service/service_test.go +++ b/internal/app/service/service_test.go @@ -352,6 +352,74 @@ func TestListContentsService(t *testing.T) { ) } +func TestListContentsWithFilter(t *testing.T) { + t.Parallel() + client, assert := setup(t) + + // Store contents with different namespaces + storeMultipleContents(storeMultipleContentsParams{ + Client: client, Assert: assert, Count: 5, + Name: "catalog", Namespace: "dsc", + }) + storeMultipleContents(storeMultipleContentsParams{ + Client: client, Assert: assert, Count: 5, + Name: "page", Namespace: "dicty", + }) + + testCases := []struct { + name string + filter string + check func(*require.Assertions, []*content.ContentCollection_Data) + }{ + { + name: "Filter by namespace", + filter: "namespace===dsc", + check: func(assert *require.Assertions, data []*content.ContentCollection_Data) { + for _, item := range data { + assert.Equal(item.Attributes.Namespace, "dsc") + } + }, + }, + { + name: "Filter by name", + filter: "name=~page", + check: func(assert *require.Assertions, data []*content.ContentCollection_Data) { + for _, item := range data { + assert.Contains(item.Attributes.Name, "page") + } + }, + }, + { + name: "Combined filter", + filter: "namespace===dicty;name=~page", + check: func(assert *require.Assertions, data []*content.ContentCollection_Data) { + for _, item := range data { + assert.Equal(item.Attributes.Namespace, "dicty") + assert.Contains(item.Attributes.Name, "page") + } + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + resp, err := client.ListContents(context.Background(), &content.ListParameters{ + Limit: 10, Filter: tc.filter, + }) + assert.NoError(err, "expect no error from listing contents with filter") + assert.Len(resp.Data, 5, "should return 5 contents") + tc.check(assert, resp.Data) + }) + } + + // Test filter with no results + _, err := client.ListContents(context.Background(), &content.ListParameters{ + Limit: 10, Filter: "namespace===nonexistent", + }) + assert.Error(err, "expect an error when no results are found") + assert.Equal(codes.NotFound, status.Code(err), "error should be NotFound") +} + func validateListContents(params validateListContentsParams) { for _, cnt := range params.Data { params.Assert.Regexp( From 2d55411e38d26769682e78cf67d1ad82a1209f37 Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Fri, 16 Aug 2024 11:36:06 -0500 Subject: [PATCH 33/34] test(service_test.go): rename and reformat test function for clarity and readability The test function name is updated from `TestListContentsWithFilter` to `TestListContentsServiceWithFilter` to better reflect its purpose and maintain consistency with other test function naming conventions in the service tests. Additionally, the function call and error assertion are reformatted to improve readability, making it easier to understand the test logic and parameters involved. --- internal/app/service/service_test.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/internal/app/service/service_test.go b/internal/app/service/service_test.go index 62130a5..7636cae 100644 --- a/internal/app/service/service_test.go +++ b/internal/app/service/service_test.go @@ -352,7 +352,7 @@ func TestListContentsService(t *testing.T) { ) } -func TestListContentsWithFilter(t *testing.T) { +func TestListContentsServiceWithFilter(t *testing.T) { t.Parallel() client, assert := setup(t) @@ -403,10 +403,16 @@ func TestListContentsWithFilter(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - resp, err := client.ListContents(context.Background(), &content.ListParameters{ - Limit: 10, Filter: tc.filter, - }) - assert.NoError(err, "expect no error from listing contents with filter") + resp, err := client.ListContents( + context.Background(), + &content.ListParameters{ + Limit: 10, Filter: tc.filter, + }, + ) + assert.NoError( + err, + "expect no error from listing contents with filter", + ) assert.Len(resp.Data, 5, "should return 5 contents") tc.check(assert, resp.Data) }) From 0e144fd75777083b4514465f4c590b1492c44fad Mon Sep 17 00:00:00 2001 From: Siddhartha Basu Date: Fri, 16 Aug 2024 11:46:02 -0500 Subject: [PATCH 34/34] feat(service_test.go): enhance ListContentsService tests with structured scenarios The test for ListContentsService is refactored to include multiple test cases, covering basic listing, listing with a cursor, listing with a filter, and a combination of cursor and filter. This structured approach allows for more comprehensive testing of the service's behavior under various conditions, ensuring that the service can handle different types of requests correctly. --- internal/app/service/service_test.go | 154 ++++++++++++++++++++------- 1 file changed, 117 insertions(+), 37 deletions(-) diff --git a/internal/app/service/service_test.go b/internal/app/service/service_test.go index 7636cae..2169a35 100644 --- a/internal/app/service/service_test.go +++ b/internal/app/service/service_test.go @@ -298,58 +298,138 @@ func TestListContentsService(t *testing.T) { name, namespace := "catalog", "dsc" storeMultipleContents(storeMultipleContentsParams{ - Client: client, - Assert: assert, - Count: 10, - Name: name, - Namespace: namespace, + Client: client, Assert: assert, Count: 20, Name: name, Namespace: namespace, }) - resp, err := client.ListContents( - context.Background(), - &content.ListParameters{Limit: 5}, - ) - assert.NoError(err, "expect no error from listing contents") + + testCases := []struct { + name string + params *content.ListParameters + validate func(*testing.T, *content.ContentCollection, content.ContentServiceClient) + }{ + { + name: "Basic listing", + params: &content.ListParameters{Limit: 5}, + validate: validateBasicListing, + }, + { + name: "Listing with cursor", + params: &content.ListParameters{Limit: 7}, + validate: validateListingWithCursor, + }, + { + name: "Listing with filter", + params: &content.ListParameters{ + Limit: 10, + Filter: fmt.Sprintf("namespace===%s", namespace), + }, + validate: validateListingWithFilter, + }, + { + name: "Listing with cursor and filter", + params: &content.ListParameters{ + Limit: 5, + Filter: fmt.Sprintf("namespace===%s", namespace), + }, + validate: validateListingWithCursorAndFilter, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + resp, err := client.ListContents(context.Background(), tc.params) + assert.NoError(err, "expect no error from listing contents") + tc.validate(t, resp, client) + }) + } +} + +func validateBasicListing( + t *testing.T, + resp *content.ContentCollection, + _ content.ContentServiceClient, +) { + assert := require.New(t) assert.Len(resp.Data, 5, "should return 5 contents") - nameRegex := regexp.MustCompile(fmt.Sprintf(`%s-\d+`, name)) + nameRegex := regexp.MustCompile(`catalog-\d+`) validateListContents(validateListContentsParams{ - Assert: assert, - Data: resp.Data, - NameRegex: nameRegex, - Namespace: namespace, + Assert: assert, Data: resp.Data, NameRegex: nameRegex, Namespace: "dsc", }) assert.Greater( resp.Meta.NextCursor, int64(0), - "should have more record to fetch", + "should have more records to fetch", ) +} + +func validateListingWithCursor( + t *testing.T, + resp1 *content.ContentCollection, + client content.ContentServiceClient, +) { + assert := require.New(t) + assert.Len(resp1.Data, 7, "should return 7 contents") resp2, err := client.ListContents( context.Background(), - &content.ListParameters{Limit: 7, Cursor: resp.Meta.NextCursor}, + &content.ListParameters{ + Limit: 7, + Cursor: resp1.Meta.NextCursor, + }, ) - assert.NoError(err, "expect no error from listing contents") - assert.Len(resp2.Data, 5, "should return 5 contents") - validateListContents(validateListContentsParams{ - Assert: assert, - Data: resp2.Data, - NameRegex: nameRegex, - Namespace: namespace, - }) - assert.Equal( - resp2.Meta.NextCursor, - int64(0), - "should not have any more record to fetch", + assert.NoError(err, "expect no error from second listing") + assert.Len(resp2.Data, 7, "should return 7 contents") + assert.NotEqual( + resp1.Data[0].Id, + resp2.Data[0].Id, + "first items should be different", ) - resp2, err = client.ListContents( +} + +func validateListingWithFilter( + t *testing.T, + resp *content.ContentCollection, + _ content.ContentServiceClient, +) { + assert := require.New(t) + assert.NotEmpty(resp.Data, "should return some contents") + for _, item := range resp.Data { + assert.Equal( + "dsc", + item.Attributes.Namespace, + "all items should have the filtered namespace", + ) + } +} + +func validateListingWithCursorAndFilter( + t *testing.T, + resp1 *content.ContentCollection, + client content.ContentServiceClient, +) { + assert := require.New(t) + assert.Len(resp1.Data, 5, "should return 5 contents") + resp2, err := client.ListContents( context.Background(), - &content.ListParameters{Limit: 10}, + &content.ListParameters{ + Limit: 5, Cursor: resp1.Meta.NextCursor, Filter: "namespace===dsc", + }, ) - assert.NoError(err, "expect no error from listing contents") - assert.Len(resp2.Data, 10, "should return 10 contents") - assert.Equal( - resp2.Meta.NextCursor, - int64(0), - "should not have any more record to fetch", + assert.NoError( + err, + "expect no error from second listing with filter and cursor", ) + assert.Len(resp2.Data, 5, "should return 5 contents") + assert.NotEqual( + resp1.Data[0].Id, + resp2.Data[0].Id, + "first items should be different", + ) + for _, item := range resp2.Data { + assert.Equal( + "dsc", + item.Attributes.Namespace, + "all items should have the filtered namespace", + ) + } } func TestListContentsServiceWithFilter(t *testing.T) {