Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to use optional.Option for issue index search option #29739

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions modules/indexer/internal/bleve/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package bleve

import (
"code.gitea.io/gitea/modules/optional"

"github.com/blevesearch/bleve/v2"
"github.com/blevesearch/bleve/v2/search/query"
)
Expand Down Expand Up @@ -39,18 +41,18 @@ func BoolFieldQuery(value bool, field string) *query.BoolFieldQuery {
return q
}

func NumericRangeInclusiveQuery(min, max *int64, field string) *query.NumericRangeQuery {
func NumericRangeInclusiveQuery(min, max optional.Option[int64], field string) *query.NumericRangeQuery {
var minF, maxF *float64
var minI, maxI *bool
if min != nil {
if min.Has() {
minF = new(float64)
*minF = float64(*min)
*minF = float64(min.Value())
minI = new(bool)
*minI = true
}
if max != nil {
if max.Has() {
maxF = new(float64)
*maxF = float64(*max)
*maxF = float64(max.Value())
maxI = new(bool)
*maxI = true
}
Expand Down
39 changes: 21 additions & 18 deletions modules/indexer/issues/bleve/bleve.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,38 +224,41 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) (
queries = append(queries, bleve.NewDisjunctionQuery(milestoneQueries...))
}

if options.ProjectID != nil {
queries = append(queries, inner_bleve.NumericEqualityQuery(*options.ProjectID, "project_id"))
if options.ProjectID.Has() {
queries = append(queries, inner_bleve.NumericEqualityQuery(options.ProjectID.Value(), "project_id"))
}
if options.ProjectBoardID != nil {
queries = append(queries, inner_bleve.NumericEqualityQuery(*options.ProjectBoardID, "project_board_id"))
if options.ProjectBoardID.Has() {
queries = append(queries, inner_bleve.NumericEqualityQuery(options.ProjectBoardID.Value(), "project_board_id"))
}

if options.PosterID != nil {
queries = append(queries, inner_bleve.NumericEqualityQuery(*options.PosterID, "poster_id"))
if options.PosterID.Has() {
queries = append(queries, inner_bleve.NumericEqualityQuery(options.PosterID.Value(), "poster_id"))
}

if options.AssigneeID != nil {
queries = append(queries, inner_bleve.NumericEqualityQuery(*options.AssigneeID, "assignee_id"))
if options.AssigneeID.Has() {
queries = append(queries, inner_bleve.NumericEqualityQuery(options.AssigneeID.Value(), "assignee_id"))
}

if options.MentionID != nil {
queries = append(queries, inner_bleve.NumericEqualityQuery(*options.MentionID, "mention_ids"))
if options.MentionID.Has() {
queries = append(queries, inner_bleve.NumericEqualityQuery(options.MentionID.Value(), "mention_ids"))
}

if options.ReviewedID != nil {
queries = append(queries, inner_bleve.NumericEqualityQuery(*options.ReviewedID, "reviewed_ids"))
if options.ReviewedID.Has() {
queries = append(queries, inner_bleve.NumericEqualityQuery(options.ReviewedID.Value(), "reviewed_ids"))
}
if options.ReviewRequestedID != nil {
queries = append(queries, inner_bleve.NumericEqualityQuery(*options.ReviewRequestedID, "review_requested_ids"))
if options.ReviewRequestedID.Has() {
queries = append(queries, inner_bleve.NumericEqualityQuery(options.ReviewRequestedID.Value(), "review_requested_ids"))
}

if options.SubscriberID != nil {
queries = append(queries, inner_bleve.NumericEqualityQuery(*options.SubscriberID, "subscriber_ids"))
if options.SubscriberID.Has() {
queries = append(queries, inner_bleve.NumericEqualityQuery(options.SubscriberID.Value(), "subscriber_ids"))
}

if options.UpdatedAfterUnix != nil || options.UpdatedBeforeUnix != nil {
queries = append(queries, inner_bleve.NumericRangeInclusiveQuery(options.UpdatedAfterUnix, options.UpdatedBeforeUnix, "updated_unix"))
if options.UpdatedAfterUnix.Has() || options.UpdatedBeforeUnix.Has() {
queries = append(queries, inner_bleve.NumericRangeInclusiveQuery(
options.UpdatedAfterUnix,
options.UpdatedBeforeUnix,
"updated_unix"))
}

var indexerQuery query.Query = bleve.NewConjunctionQuery(queries...)
Expand Down
32 changes: 14 additions & 18 deletions modules/indexer/issues/db/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,6 @@ 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
}
if *id == 0 {
return db.NoConditionID
}
return *id
}
convertInt64 := func(i *int64) int64 {
if i == nil {
return 0
}
return *i
}
var sortType string
switch options.SortBy {
case internal.SortByCreatedAsc:
Expand All @@ -53,6 +37,18 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m
sortType = "newest"
}

// See the comment of issues_model.SearchOptions for the reason why we need to convert
convertID := func(id optional.Option[int64]) int64 {
if !id.Has() {
return 0
}
value := id.Value()
if value == 0 {
return db.NoConditionID
}
return value
}

opts := &issue_model.IssuesOptions{
Paginator: options.Paginator,
RepoIDs: options.RepoIDs,
Expand All @@ -73,8 +69,8 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m
IncludeMilestones: nil,
SortType: sortType,
IssueIDs: nil,
UpdatedAfterUnix: convertInt64(options.UpdatedAfterUnix),
UpdatedBeforeUnix: convertInt64(options.UpdatedBeforeUnix),
UpdatedAfterUnix: options.UpdatedAfterUnix.Value(),
UpdatedBeforeUnix: options.UpdatedBeforeUnix.Value(),
PriorityRepoID: 0,
IsArchived: optional.None[bool](),
Org: nil,
Expand Down
12 changes: 6 additions & 6 deletions modules/indexer/issues/dboptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package issues
import (
"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/modules/optional"
)

func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOptions {
Expand Down Expand Up @@ -38,13 +39,12 @@ func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOp
}

// See the comment of issues_model.SearchOptions for the reason why we need to convert
convertID := func(id int64) *int64 {
convertID := func(id int64) optional.Option[int64] {
if id > 0 {
return &id
return optional.Some(id)
}
if id == db.NoConditionID {
var zero int64
return &zero
return optional.None[int64]()
}
return nil
}
Expand All @@ -59,10 +59,10 @@ func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOp
searchOpt.SubscriberID = convertID(opts.SubscriberID)

if opts.UpdatedAfterUnix > 0 {
searchOpt.UpdatedAfterUnix = &opts.UpdatedAfterUnix
searchOpt.UpdatedAfterUnix = optional.Some(opts.UpdatedAfterUnix)
}
if opts.UpdatedBeforeUnix > 0 {
searchOpt.UpdatedBeforeUnix = &opts.UpdatedBeforeUnix
searchOpt.UpdatedBeforeUnix = optional.Some(opts.UpdatedBeforeUnix)
}

searchOpt.Paginator = opts.Paginator
Expand Down
42 changes: 21 additions & 21 deletions modules/indexer/issues/elasticsearch/elasticsearch.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,43 +195,43 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) (
query.Must(elastic.NewTermsQuery("milestone_id", toAnySlice(options.MilestoneIDs)...))
}

if options.ProjectID != nil {
query.Must(elastic.NewTermQuery("project_id", *options.ProjectID))
if options.ProjectID.Has() {
query.Must(elastic.NewTermQuery("project_id", options.ProjectID.Value()))
}
if options.ProjectBoardID != nil {
query.Must(elastic.NewTermQuery("project_board_id", *options.ProjectBoardID))
if options.ProjectBoardID.Has() {
query.Must(elastic.NewTermQuery("project_board_id", options.ProjectBoardID.Value()))
}

if options.PosterID != nil {
query.Must(elastic.NewTermQuery("poster_id", *options.PosterID))
if options.PosterID.Has() {
query.Must(elastic.NewTermQuery("poster_id", options.PosterID.Value()))
}

if options.AssigneeID != nil {
query.Must(elastic.NewTermQuery("assignee_id", *options.AssigneeID))
if options.AssigneeID.Has() {
query.Must(elastic.NewTermQuery("assignee_id", options.AssigneeID.Value()))
}

if options.MentionID != nil {
query.Must(elastic.NewTermQuery("mention_ids", *options.MentionID))
if options.MentionID.Has() {
query.Must(elastic.NewTermQuery("mention_ids", options.MentionID.Value()))
}

if options.ReviewedID != nil {
query.Must(elastic.NewTermQuery("reviewed_ids", *options.ReviewedID))
if options.ReviewedID.Has() {
query.Must(elastic.NewTermQuery("reviewed_ids", options.ReviewedID.Value()))
}
if options.ReviewRequestedID != nil {
query.Must(elastic.NewTermQuery("review_requested_ids", *options.ReviewRequestedID))
if options.ReviewRequestedID.Has() {
query.Must(elastic.NewTermQuery("review_requested_ids", options.ReviewRequestedID.Value()))
}

if options.SubscriberID != nil {
query.Must(elastic.NewTermQuery("subscriber_ids", *options.SubscriberID))
if options.SubscriberID.Has() {
query.Must(elastic.NewTermQuery("subscriber_ids", options.SubscriberID.Value()))
}

if options.UpdatedAfterUnix != nil || options.UpdatedBeforeUnix != nil {
if options.UpdatedAfterUnix.Has() || options.UpdatedBeforeUnix.Has() {
q := elastic.NewRangeQuery("updated_unix")
if options.UpdatedAfterUnix != nil {
q.Gte(*options.UpdatedAfterUnix)
if options.UpdatedAfterUnix.Has() {
q.Gte(options.UpdatedAfterUnix.Value())
}
if options.UpdatedBeforeUnix != nil {
q.Lte(*options.UpdatedBeforeUnix)
if options.UpdatedBeforeUnix.Has() {
q.Lte(options.UpdatedBeforeUnix.Value())
}
query.Must(q)
}
Expand Down
65 changes: 28 additions & 37 deletions modules/indexer/issues/indexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,63 +134,60 @@ func searchIssueInRepo(t *testing.T) {
}

func searchIssueByID(t *testing.T) {
int64Pointer := func(x int64) *int64 {
return &x
}
tests := []struct {
opts SearchOptions
expectedIDs []int64
}{
{
SearchOptions{
PosterID: int64Pointer(1),
opts: SearchOptions{
PosterID: optional.Some(int64(1)),
},
[]int64{11, 6, 3, 2, 1},
expectedIDs: []int64{11, 6, 3, 2, 1},
},
{
SearchOptions{
AssigneeID: int64Pointer(1),
opts: SearchOptions{
AssigneeID: optional.Some(int64(1)),
},
[]int64{6, 1},
expectedIDs: []int64{6, 1},
},
{
SearchOptions{
MentionID: int64Pointer(4),
opts: SearchOptions{
MentionID: optional.Some(int64(4)),
},
[]int64{1},
expectedIDs: []int64{1},
},
{
SearchOptions{
ReviewedID: int64Pointer(1),
opts: SearchOptions{
ReviewedID: optional.Some(int64(1)),
},
[]int64{},
expectedIDs: []int64{},
},
{
SearchOptions{
ReviewRequestedID: int64Pointer(1),
opts: SearchOptions{
ReviewRequestedID: optional.Some(int64(1)),
},
[]int64{12},
expectedIDs: []int64{12},
},
{
SearchOptions{
SubscriberID: int64Pointer(1),
opts: SearchOptions{
SubscriberID: optional.Some(int64(1)),
},
[]int64{11, 6, 5, 3, 2, 1},
expectedIDs: []int64{11, 6, 5, 3, 2, 1},
},
{
// issue 20 request user 15 and team 5 which user 15 belongs to
// the review request number of issue 20 should be 1
SearchOptions{
ReviewRequestedID: int64Pointer(15),
opts: SearchOptions{
ReviewRequestedID: optional.Some(int64(15)),
},
[]int64{12, 20},
expectedIDs: []int64{12, 20},
},
{
// user 20 approved the issue 20, so return nothing
SearchOptions{
ReviewRequestedID: int64Pointer(20),
opts: SearchOptions{
ReviewRequestedID: optional.Some(int64(20)),
},
[]int64{},
expectedIDs: []int64{},
},
}

Expand Down Expand Up @@ -318,16 +315,13 @@ func searchIssueByLabelID(t *testing.T) {
}

func searchIssueByTime(t *testing.T) {
int64Pointer := func(i int64) *int64 {
return &i
}
tests := []struct {
opts SearchOptions
expectedIDs []int64
}{
{
SearchOptions{
UpdatedAfterUnix: int64Pointer(0),
UpdatedAfterUnix: optional.Some(int64(0)),
},
[]int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 1},
},
Expand Down Expand Up @@ -363,28 +357,25 @@ func searchIssueWithOrder(t *testing.T) {
}

func searchIssueInProject(t *testing.T) {
int64Pointer := func(i int64) *int64 {
return &i
}
tests := []struct {
opts SearchOptions
expectedIDs []int64
}{
{
SearchOptions{
ProjectID: int64Pointer(1),
ProjectID: optional.Some(int64(1)),
},
[]int64{5, 3, 2, 1},
},
{
SearchOptions{
ProjectBoardID: int64Pointer(1),
ProjectBoardID: optional.Some(int64(1)),
},
[]int64{1},
},
{
SearchOptions{
ProjectBoardID: int64Pointer(0), // issue with in default board
ProjectBoardID: optional.Some(int64(0)), // issue with in default board
},
[]int64{2},
},
Expand Down
Loading