Explain SearchOptions and fix ToSearchOptions (#26542)
Follow #26012 #26490. A detailed description has been added to the comment.
This commit is contained in:
parent
1432d4eab9
commit
3b129aaa80
|
@ -14,6 +14,7 @@ import (
|
|||
)
|
||||
|
||||
func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_model.IssuesOptions, error) {
|
||||
// See the comment of issues_model.SearchOptions for the reason why we need to convert
|
||||
convertID := func(id *int64) int64 {
|
||||
if id == nil {
|
||||
return 0
|
||||
|
|
|
@ -17,10 +17,6 @@ func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOp
|
|||
IsClosed: opts.IsClosed,
|
||||
}
|
||||
|
||||
if opts.ProjectID != 0 {
|
||||
searchOpt.ProjectID = &opts.ProjectID
|
||||
}
|
||||
|
||||
if len(opts.LabelIDs) == 1 && opts.LabelIDs[0] == 0 {
|
||||
searchOpt.NoLabelOnly = true
|
||||
} else {
|
||||
|
@ -41,25 +37,27 @@ func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOp
|
|||
searchOpt.MilestoneIDs = opts.MilestoneIDs
|
||||
}
|
||||
|
||||
if opts.AssigneeID > 0 {
|
||||
searchOpt.AssigneeID = &opts.AssigneeID
|
||||
}
|
||||
if opts.PosterID > 0 {
|
||||
searchOpt.PosterID = &opts.PosterID
|
||||
}
|
||||
if opts.MentionedID > 0 {
|
||||
searchOpt.MentionID = &opts.MentionedID
|
||||
}
|
||||
if opts.ReviewedID > 0 {
|
||||
searchOpt.ReviewedID = &opts.ReviewedID
|
||||
}
|
||||
if opts.ReviewRequestedID > 0 {
|
||||
searchOpt.ReviewRequestedID = &opts.ReviewRequestedID
|
||||
}
|
||||
if opts.SubscriberID > 0 {
|
||||
searchOpt.SubscriberID = &opts.SubscriberID
|
||||
// See the comment of issues_model.SearchOptions for the reason why we need to convert
|
||||
convertID := func(id int64) *int64 {
|
||||
if id > 0 {
|
||||
return &id
|
||||
}
|
||||
if id == db.NoConditionID {
|
||||
var zero int64
|
||||
return &zero
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
searchOpt.ProjectID = convertID(opts.ProjectID)
|
||||
searchOpt.ProjectBoardID = convertID(opts.ProjectBoardID)
|
||||
searchOpt.PosterID = convertID(opts.PosterID)
|
||||
searchOpt.AssigneeID = convertID(opts.AssigneeID)
|
||||
searchOpt.MentionID = convertID(opts.MentionedID)
|
||||
searchOpt.ReviewedID = convertID(opts.ReviewedID)
|
||||
searchOpt.ReviewRequestedID = convertID(opts.ReviewRequestedID)
|
||||
searchOpt.SubscriberID = convertID(opts.SubscriberID)
|
||||
|
||||
if opts.UpdatedAfterUnix > 0 {
|
||||
searchOpt.UpdatedAfterUnix = &opts.UpdatedAfterUnix
|
||||
}
|
||||
|
|
|
@ -56,7 +56,21 @@ type SearchResult struct {
|
|||
Hits []Match
|
||||
}
|
||||
|
||||
// SearchOptions represents search options
|
||||
// SearchOptions represents search options.
|
||||
//
|
||||
// It has a slightly different design from database query options.
|
||||
// In database query options, a field is never a pointer, so it could be confusing when it's zero value:
|
||||
// Do you want to find data with a field value of 0, or do you not specify the field in the options?
|
||||
// To avoid this confusion, db introduced db.NoConditionID(-1).
|
||||
// So zero value means the field is not specified in the search options, and db.NoConditionID means "== 0" or "id NOT IN (SELECT id FROM ...)"
|
||||
// It's still not ideal, it trapped developers many times.
|
||||
// And sometimes -1 could be a valid value, like issue ID, negative numbers indicate exclusion.
|
||||
// Since db.NoConditionID is for "db" (the package name is db), it makes sense not to use it in the indexer:
|
||||
// Why do bleve/elasticsearch/meilisearch indexers need to know about db.NoConditionID?
|
||||
// So in SearchOptions, we use pointer for fields which could be not specified,
|
||||
// and always use the value to filter if it's not nil, even if it's zero or negative.
|
||||
// It can handle almost all cases, if there is an exception, we can add a new field, like NoLabelOnly.
|
||||
// Unfortunately, we still use db for the indexer and have to convert between db.NoConditionID and nil for legacy reasons.
|
||||
type SearchOptions struct {
|
||||
Keyword string // keyword to search
|
||||
|
||||
|
|
Loading…
Reference in a new issue