Auto-generated Pull Request for bugfix/aql-bind-parameter-listannotations

internal/app/service/feature_annotation.go

@@ -281,6 +281,7 @@ func (srv *FeatureAnnotationService) RemoveTags(
 }
 
 // ListFeatureAnnotationsByPubmedId lists all feature annotations associated with a PubMed ID.
+//
 //nolint:staticcheck // must match gRPC-generated interface name
 func (srv *FeatureAnnotationService) ListFeatureAnnotationsByPubmedId(
 	ctx context.Context,

internal/repository/arangodb/annotation_read.go

@@ -74,29 +74,54 @@ func (ar *arangorepository) GetAnnotationByEntry(
 	return mann, nil
 }
 
+// buildListAnnoBindVars constructs the AQL bind variable map for
+// ListAnnotations. The set of collection bind parameters depends on the
+// statement type: SecondFilter queries traverse cvterm first (no
+// @@anno_collection), while all other types traverse annotations first (no
+// @@cvterm_collection). Providing an unreferenced key causes ArangoDB to
+// reject the query, so the two collection parameters are mutually exclusive.
+func buildListAnnoBindVars(
+	stmtType StatementType,
+	annoCollection, cvCollection, cvtermCollection, graphName string,
+	limit, cursor int64,
+) map[string]any {
+	bindVars := map[string]any{
+		cvCollectionBind:    cvCollection,
+		"anno_cvterm_graph": graphName,
+		"limit":             limit + 1,
+	}
+	if cursor != 0 {
+		bindVars["cursor"] = cursor
+	}
+	if stmtType == SecondFilter {
+		bindVars[cvtermCollectionBind] = cvtermCollection
+	} else {
+		bindVars["@anno_collection"] = annoCollection
+	}
+
+	return bindVars
+}
+
 func (ar *arangorepository) ListAnnotations(
 	params *repository.ListAnnotationsParams,
 ) ([]*model.AnnoDoc, error) {
 	if err := validate.Struct(params); err != nil {
 		return nil, fmt.Errorf("error in valdating parameters %w", err)
 	}
 	annoModel := make([]*model.AnnoDoc, 0)
-	bindVars := map[string]any{
-		"@anno_collection":  ar.anno.annot.Name(),
-		cvCollectionBind:    ar.onto.Cv.Name(),
-		"anno_cvterm_graph": ar.anno.annotg.Name(),
-		"limit":             params.Limit + 1,
-	}
-	if params.Cursor != 0 {
-		bindVars["cursor"] = params.Cursor
-	}
 	result := getListAnnoStatement(params.Filter, params.Cursor)
-	if result.Type == SecondFilter {
-		bindVars[cvtermCollectionBind] = ar.onto.Term.Name()
-	}
 	if result.Err != nil {
 		return nil, result.Err
 	}
+	bindVars := buildListAnnoBindVars(
+		result.Type,
+		ar.anno.annot.Name(),
+		ar.onto.Cv.Name(),
+		ar.onto.Term.Name(),
+		ar.anno.annotg.Name(),
+		params.Limit,
+		params.Cursor,
+	)
 	res, err := ar.database.SearchRows(result.Statement, bindVars)
 	if err != nil {
 		return annoModel, fmt.Errorf("error in searching rows %s", err)

internal/repository/arangodb/annotation_read_bind_test.go

@@ -0,0 +1,93 @@
+package arangodb
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestBuildListAnnoBindVars(t *testing.T) {
+	t.Parallel()
+	const (
+		anno   = "annotation"
+		cv     = "cv"
+		cvterm = "cvterm"
+		graph  = "anno_cvterm_graph"
+		limit  = int64(10)
+	)
+
+	t.Run(
+		"FirstFilter includes anno_collection and excludes cvterm_collection",
+		func(t *testing.T) {
+			t.Parallel()
+			assert := require.New(t)
+			vars := buildListAnnoBindVars(FirstFilter, anno, cv, cvterm, graph, limit, 0)
+			assert.Equal(anno, vars["@anno_collection"])
+			assert.Equal(cv, vars[cvCollectionBind])
+			assert.NotContains(vars, cvtermCollectionBind)
+			assert.NotContains(vars, "cursor")
+			assert.Equal(limit+1, vars["limit"])
+		},
+	)
+
+	t.Run(
+		"SecondFilter excludes anno_collection and includes cvterm_collection",
+		func(t *testing.T) {
+			t.Parallel()
+			assert := require.New(t)
+			vars := buildListAnnoBindVars(SecondFilter, anno, cv, cvterm, graph, limit, 0)
+			assert.NotContains(vars, "@anno_collection")
+			assert.Equal(cv, vars[cvCollectionBind])
+			assert.Equal(cvterm, vars[cvtermCollectionBind])
+			assert.NotContains(vars, "cursor")
+		},
+	)
+
+	t.Run(
+		"BothFilters includes anno_collection and excludes cvterm_collection",
+		func(t *testing.T) {
+			t.Parallel()
+			assert := require.New(t)
+			vars := buildListAnnoBindVars(BothFilters, anno, cv, cvterm, graph, limit, 0)
+			assert.Equal(anno, vars["@anno_collection"])
+			assert.Equal(cv, vars[cvCollectionBind])
+			assert.NotContains(vars, cvtermCollectionBind)
+		},
+	)
+
+	t.Run("cursor key is present when cursor is non-zero", func(t *testing.T) {
+		t.Parallel()
+		assert := require.New(t)
+		vars := buildListAnnoBindVars(FirstFilter, anno, cv, cvterm, graph, limit, 12345)
+		assert.Equal(int64(12345), vars["cursor"])
+	})
+
+	t.Run("cursor key is absent when cursor is zero", func(t *testing.T) {
+		t.Parallel()
+		assert := require.New(t)
+		vars := buildListAnnoBindVars(SecondFilter, anno, cv, cvterm, graph, limit, 0)
+		assert.NotContains(vars, "cursor")
+	})
+
+	t.Run(
+		"SecondFilter with cursor includes cvterm_collection and cursor but not anno_collection",
+		func(t *testing.T) {
+			t.Parallel()
+			assert := require.New(t)
+			vars := buildListAnnoBindVars(SecondFilter, anno, cv, cvterm, graph, 5, 99999)
+			assert.NotContains(vars, "@anno_collection")
+			assert.Equal(cvterm, vars[cvtermCollectionBind])
+			assert.Equal(int64(99999), vars["cursor"])
+			assert.Equal(int64(6), vars["limit"])
+		},
+	)
+
+	t.Run("anno_cvterm_graph is always present", func(t *testing.T) {
+		t.Parallel()
+		assert := require.New(t)
+		for _, stype := range []StatementType{FirstFilter, SecondFilter, BothFilters} {
+			vars := buildListAnnoBindVars(stype, anno, cv, cvterm, graph, limit, 0)
+			assert.Equal(graph, vars["anno_cvterm_graph"], "graph missing for type %s", stype)
+		}
+	})
+}

internal/repository/arangodb/annotation_read_test.go

@@ -10,9 +10,10 @@ import (
 )
 
 const (
-	filterOne   = `entry_id==DDB_G0286429;tag==private note;ontology==dicty_annotation`
-	filterTwo   = `entry_id==DDB_G0294491;tag==name description;ontology==dicty_annotation`
-	filterThree = `entry_id==jumbo`
+	filterOne     = `entry_id==DDB_G0286429;tag==private note;ontology==dicty_annotation`
+	filterTwo     = `entry_id==DDB_G0294491;tag==name description;ontology==dicty_annotation`
+	filterThree   = `entry_id==jumbo`
+	filterTagOnly = `tag==private note;ontology==dicty_annotation`
 )
 
 //nolint:tparallel
@@ -610,3 +611,95 @@ func testAddAnnotationSuccessThird(
 	)
 	assert.Equal(nta.Data.Attributes.Tag, annm.Tag, "should match the tag")
 }
+
+// TestListAnnoTagOnlyFilter exercises the SecondFilter (tag+ontology only, no
+// entry_id) code path in ListAnnotations — the path that previously failed
+// with AQL bind parameter errors.
+//
+//nolint:tparallel
+func TestListAnnoTagOnlyFilter(t *testing.T) {
+	t.Parallel()
+	assert, anrepo := setUp(t)
+	defer tearDown(anrepo)
+
+	tal := newTestTaggedAnnotationsListForFiltering(10)
+	for _, anno := range tal {
+		_, err := anrepo.AddAnnotation(anno)
+		assert.NoErrorf(err, "setup: expect no error adding annotation, received %s", err)
+	}
+
+	var firstPage []*model.AnnoDoc
+
+	t.Run("FirstPage", func(t *testing.T) {
+		firstPage = testListAnnoTagOnlyFirstPage(t, assert, anrepo)
+	})
+
+	t.Run("SecondPage", func(t *testing.T) {
+		testListAnnoTagOnlySecondPage(t, assert, anrepo, firstPage)
+	})
+
+	t.Run("NoResults", func(t *testing.T) {
+		testListAnnoTagOnlyNotFound(t, assert, anrepo)
+	})
+}
+
+func testListAnnoTagOnlyFirstPage(
+	t *testing.T,
+	assert *require.Assertions,
+	anrepo repository.TaggedAnnotationRepository,
+) []*model.AnnoDoc {
+	t.Helper()
+	mla, err := anrepo.ListAnnotations(
+		&repository.ListAnnotationsParams{Limit: 3, Filter: filterTagOnly},
+	)
+	assert.NoErrorf(err, "SecondFilter query must not return AQL bind error, got %s", err)
+	assert.NotEmpty(mla, "should return at least one annotation")
+	for _, m := range mla {
+		assert.Equal(tags[0], m.Tag, "tag should match filter")
+		assert.Equal("dicty_annotation", m.Ontology, "ontology should match filter")
+	}
+	testModelListSort(t, mla)
+
+	return mla
+}
+
+func testListAnnoTagOnlySecondPage(
+	t *testing.T,
+	assert *require.Assertions,
+	anrepo repository.TaggedAnnotationRepository,
+	prevResult []*model.AnnoDoc,
+) {
+	t.Helper()
+	assert.NotEmpty(prevResult, "previous result should not be empty")
+	ml2, err := anrepo.ListAnnotations(&repository.ListAnnotationsParams{
+		Cursor: toTimestamp(prevResult[len(prevResult)-1].CreatedAt),
+		Limit:  3,
+		Filter: filterTagOnly,
+	})
+	assert.NoErrorf(
+		err,
+		"SecondFilter+cursor query must not return AQL bind error, got %s",
+		err,
+	)
+	assert.Exactly(prevResult[len(prevResult)-1], ml2[0], "pages should overlap by one")
+	testModelListSort(t, ml2)
+}
+
+func testListAnnoTagOnlyNotFound(
+	t *testing.T,
+	assert *require.Assertions,
+	anrepo repository.TaggedAnnotationRepository,
+) {
+	t.Helper()
+	_, err := anrepo.ListAnnotations(
+		&repository.ListAnnotationsParams{
+			Limit:  4,
+			Filter: `tag==nonexistent tag;ontology==dicty_annotation`,
+		},
+	)
+	assert.Error(err, "expect not-found error")
+	assert.True(
+		repository.IsAnnotationListNotFound(err),
+		"error should be annotationListNotFound",
+	)
+}

internal/repository/arangodb/feature_annotation.go

@@ -81,8 +81,8 @@ func (fann *featureAnnoRepo) GetFeatureAnnotation(
 		featureGetByIDQ,
 		map[string]any{
 			collectionBind: fann.feature.Name(),
-			"graph":       fann.featPub.Name(),
-			"id":          fid,
+			"graph":        fann.featPub.Name(),
+			"id":           fid,
 		},
 	)
 	if err != nil {
@@ -107,8 +107,8 @@ func (fann *featureAnnoRepo) GetFeatureAnnotationByName(
 		featAnnoGetByNameQ,
 		map[string]any{
 			collectionBind: fann.feature.Name(),
-			"graph":       fann.featPub.Name(), // Add graph name
-			"name":        name,
+			"graph":        fann.featPub.Name(), // Add graph name
+			"name":         name,
 		},
 	)
 	if err != nil {
@@ -343,9 +343,9 @@ func (fann *featureAnnoRepo) ListByPublicationID(
 ) ([]*model.FeatureAnnotationDoc, error) {
 	binds := map[string]any{
 		collectionBind: fann.pub.Name(),
-		"graph":       fann.featPub.Name(),
-		"id":          publicationID,
-		"source":      source,
+		"graph":        fann.featPub.Name(),
+		"id":           publicationID,
+		"source":       source,
 	}
 
 	resultSet, err := fann.database.SearchRows(featureByPublicationIDQ, binds)
@@ -385,7 +385,7 @@ func (fann *featureAnnoRepo) RemoveFeatureAnnotation(
 ) error {
 	bindVars := map[string]any{
 		collectionBind: fann.feature.Name(),
-		"id":          fid,
+		"id":           fid,
 	}
 
 	// Check if document exists
@@ -488,8 +488,8 @@ func (fann *featureAnnoRepo) AddTags(
 	}
 	result, err := txr.DoRun(featurePropsAppendQ, map[string]any{
 		collectionBind: fann.feature.Name(),
-		"key":         doc.Key,
-		"newprops":    collection.Map(req.Tags, convertNewTagToModel),
+		"key":          doc.Key,
+		"newprops":     collection.Map(req.Tags, convertNewTagToModel),
 	})
 	if err != nil {
 		if abortErr := txr.Abort(); abortErr != nil {
@@ -542,8 +542,8 @@ func (fann *featureAnnoRepo) SetTags(
 
 	result, err := txr.DoRun(featurePropsSetQ, map[string]any{
 		collectionBind: fann.feature.Name(),
-		"key":         doc.Key,
-		"newprops":    collection.Map(req.Tags, convertNewTagToModel),
+		"key":          doc.Key,
+		"newprops":     collection.Map(req.Tags, convertNewTagToModel),
 	})
 	if err != nil {
 		if abortErr := txr.Abort(); abortErr != nil {
@@ -615,7 +615,7 @@ func (fann *featureAnnoRepo) upsertPublicationsTx(
 	result, err := txr.DoRun(
 		pubUpsertQ,
 		map[string]any{
-			"ids":         ids,
+			"ids":          ids,
 			collectionBind: fann.pub.Name(),
 		},
 	)

internal/repository/arangodb/organism.go

@@ -93,8 +93,8 @@ func (org *organismRepo) GetOrganismByName(
 	res, err := org.database.GetRow(orgGetByNameQ,
 		map[string]any{
 			collectionBind: org.organism.Name(),
-			"genus":       genus,
-			"species":     species,
+			"genus":        genus,
+			"species":      species,
 		})
 	if err != nil {
 		return nil, fmt.Errorf("error executing query: %w", err)