Skip to content

Commit 4423880

Browse files
committed
feat(search): support negating a RecordQuery for exclude filters
Signed-off-by: András Jáky <ajaky@cisco.com>
1 parent a37aba3 commit 4423880

12 files changed

Lines changed: 1035 additions & 83 deletions

File tree

api/search/v1/record_query.pb.go

Lines changed: 67 additions & 55 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

proto/agntcy/dir/search/v1/record_query.proto

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ message RecordQuery {
2121
// '*' - matches zero or more characters
2222
// '?' - matches exactly one character
2323
string value = 2;
24+
25+
// When true, exclude records matching this query instead of including them.
26+
// Default false preserves current (inclusion) behavior.
27+
bool negate = 3;
2428
}
2529

2630
// Defines a list of supported record query types.

server/controller/search_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,22 @@ func TestCountRecords_DatabaseError(t *testing.T) {
7272
require.Error(t, err)
7373
assert.ErrorContains(t, err, "failed to count records")
7474
}
75+
76+
func TestCountRecords_NegatedQuery(t *testing.T) {
77+
db := &fakeSearchDB{totalCount: 2}
78+
ctrl := NewSearchController(db, nil)
79+
80+
resp, err := ctrl.CountRecords(context.Background(), &searchv1.CountRecordsRequest{
81+
Queries: []*searchv1.RecordQuery{
82+
{
83+
Type: searchv1.RecordQueryType_RECORD_QUERY_TYPE_SKILL_NAME,
84+
Value: "nlp",
85+
Negate: true,
86+
},
87+
},
88+
})
89+
require.NoError(t, err)
90+
assert.Equal(t, uint32(2), resp.GetTotalCount())
91+
assert.Equal(t, []string{"nlp"}, db.gotFilters.Excluded.SkillNames)
92+
assert.Empty(t, db.gotFilters.SkillNames)
93+
}

server/database/database_test.go

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,3 +450,154 @@ func TestGetRecordCIDs_Annotations(t *testing.T) {
450450
assert.ElementsMatch(t, []string{ownerAlice.GetCid(), envAlice.GetCid()}, cids)
451451
})
452452
}
453+
454+
func TestGetRecordCIDs_NegatedSkill(t *testing.T) {
455+
db := setupTestDB(t)
456+
seedDB(t, db)
457+
458+
// The issue's lead case: a record with skills [nlp, python]-equivalent
459+
// must not be returned merely because it also has a different skill.
460+
cids, err := db.GetRecordCIDs(types.WithoutSkillNames("natural_language_processing/*"))
461+
require.NoError(t, err)
462+
assert.NotContains(t, cids, marketingAgent.GetCid())
463+
assert.NotContains(t, cids, healthcareAgent.GetCid())
464+
assert.Contains(t, cids, codeAssistant.GetCid())
465+
}
466+
467+
func TestGetRecordCIDs_NegatedScalarField(t *testing.T) {
468+
db := setupTestDB(t)
469+
seedDB(t, db)
470+
471+
cids, err := db.GetRecordCIDs(types.WithoutNames("*cisco*"))
472+
require.NoError(t, err)
473+
assert.NotContains(t, cids, marketingAgent.GetCid())
474+
assert.Contains(t, cids, healthcareAgent.GetCid())
475+
assert.Contains(t, cids, codeAssistant.GetCid())
476+
}
477+
478+
func TestGetRecordCIDs_IncludeAndExcludeSameType(t *testing.T) {
479+
db := setupTestDB(t)
480+
seedDB(t, db)
481+
482+
// "has NLP skill AND does not have coding skill" — marketingAgent and
483+
// healthcareAgent both have NLP; only marketingAgent lacks a coding skill.
484+
cids, err := db.GetRecordCIDs(
485+
types.WithSkillNames("natural_language_processing/*"),
486+
types.WithoutSkillNames("*coding*"),
487+
)
488+
require.NoError(t, err)
489+
assert.ElementsMatch(t, []string{marketingAgent.GetCid(), healthcareAgent.GetCid()}, cids)
490+
}
491+
492+
func TestGetRecordCIDs_NegatedAnnotation(t *testing.T) {
493+
db := setupTestDB(t)
494+
495+
ownerAlice := &testRecord{
496+
cid: "bafybeigdyrztnegannotowner00000000000000000000000000000001",
497+
name: "directory.agntcy.org/test/negowner-alice",
498+
version: "1.0.0",
499+
schemaVersion: "0.8.0",
500+
createdAt: "2024-01-15T10:30:00Z",
501+
annotations: map[string]string{"owner": "alice"},
502+
}
503+
ownerBob := &testRecord{
504+
cid: "bafybeigdyrztnegannotowner00000000000000000000000000000002",
505+
name: "directory.agntcy.org/test/negowner-bob",
506+
version: "1.0.0",
507+
schemaVersion: "0.8.0",
508+
createdAt: "2024-01-15T10:30:00Z",
509+
annotations: map[string]string{"owner": "bob"},
510+
}
511+
512+
require.NoError(t, db.AddRecord(ownerAlice))
513+
require.NoError(t, db.AddRecord(ownerBob))
514+
515+
t.Run("key-only exclusion excludes any record with that key", func(t *testing.T) {
516+
cids, err := db.GetRecordCIDs(types.WithoutAnnotationKeys("owner"))
517+
require.NoError(t, err)
518+
assert.NotContains(t, cids, ownerAlice.GetCid())
519+
assert.NotContains(t, cids, ownerBob.GetCid())
520+
})
521+
522+
// Key+value exclusion compiles to a single NOT EXISTS with both
523+
// conditions AND'd (the documented locator/annotation conflation
524+
// limitation — see applyExcludedAnnotations), matching the include
525+
// path's identical per-row conjunction. It only excludes the exact
526+
// key+value pair, not every record carrying either half.
527+
t.Run("key+value exclusion only excludes the exact pair", func(t *testing.T) {
528+
cids, err := db.GetRecordCIDs(types.WithoutAnnotationKeys("owner"), types.WithoutAnnotationValues("alice"))
529+
require.NoError(t, err)
530+
assert.NotContains(t, cids, ownerAlice.GetCid())
531+
assert.Contains(t, cids, ownerBob.GetCid())
532+
})
533+
}
534+
535+
func TestGetRecordCIDs_NegatedScanSeverity(t *testing.T) {
536+
db := setupTestDB(t)
537+
seedDB(t, db)
538+
539+
require.NoError(t, db.UpsertScanReport(&gormdb.ScanReport{
540+
RecordCID: marketingAgent.GetCid(),
541+
ScannerType: "MCP",
542+
IsSafe: false,
543+
MaxSeverity: "HIGH",
544+
}))
545+
546+
cids, err := db.GetRecordCIDs(types.WithoutScanSeverities("HIGH"))
547+
require.NoError(t, err)
548+
assert.NotContains(t, cids, marketingAgent.GetCid())
549+
assert.Contains(t, cids, healthcareAgent.GetCid())
550+
assert.Contains(t, cids, codeAssistant.GetCid())
551+
}
552+
553+
// TestGetRecordCIDs_NegatedAuthors_NullSurvives guards against the bug where
554+
// applyExcludedAuthors negated records.authors with nullable=false: gorm's JSON
555+
// serializer writes a genuine SQL NULL (not an empty-array literal) for a nil
556+
// Go slice, so NOT(NULL) silently dropped every author-less record instead of
557+
// retaining it. Unlike description, no test-only NULL forcing is needed here —
558+
// simply omitting `authors` on the testRecord literal already produces a nil
559+
// slice, which AddRecord persists as SQL NULL through the JSON serializer.
560+
func TestGetRecordCIDs_NegatedAuthors_NullSurvives(t *testing.T) {
561+
db := setupTestDB(t)
562+
563+
withAuthor := &testRecord{
564+
cid: "bafybeigdyrztnegauth00000000000000000000000000000000001",
565+
name: "directory.agntcy.org/test/has-author",
566+
version: "1.0.0",
567+
schemaVersion: "0.8.0",
568+
createdAt: "2024-01-15T10:30:00Z",
569+
authors: []string{"spam@example.com"},
570+
}
571+
noAuthors := &testRecord{
572+
cid: "bafybeigdyrztnegauth00000000000000000000000000000000002",
573+
name: "directory.agntcy.org/test/no-authors",
574+
version: "1.0.0",
575+
schemaVersion: "0.8.0",
576+
createdAt: "2024-01-15T10:30:00Z",
577+
// authors intentionally left nil -> records.authors is genuine SQL NULL.
578+
}
579+
580+
require.NoError(t, db.AddRecord(withAuthor))
581+
require.NoError(t, db.AddRecord(noAuthors))
582+
583+
cids, err := db.GetRecordCIDs(types.WithoutAuthors("spam"))
584+
require.NoError(t, err)
585+
assert.NotContains(t, cids, withAuthor.GetCid())
586+
assert.Contains(t, cids, noAuthors.GetCid())
587+
}
588+
589+
func TestCountRecords_AgreesWithGetRecordCIDs_UnderExclusion(t *testing.T) {
590+
db := setupTestDB(t)
591+
seedDB(t, db)
592+
593+
opts := []types.FilterOption{types.WithoutSkillNames("natural_language_processing/*")}
594+
595+
cids, err := db.GetRecordCIDs(opts...)
596+
require.NoError(t, err)
597+
598+
count, err := db.CountRecords(opts...)
599+
require.NoError(t, err)
600+
601+
//nolint:gosec // len(cids) is bounded by database size, no overflow risk
602+
assert.Equal(t, uint32(len(cids)), count)
603+
}

server/database/gorm/record.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,8 @@ func (d *DB) handleFilterOptions(query *gorm.DB, cfg *types.RecordFilters) *gorm
637637
}
638638
}
639639

640+
query = applyExclusionFilters(query, &cfg.Excluded)
641+
640642
return query
641643
}
642644

0 commit comments

Comments
 (0)