Skip to content

Commit

Permalink
Refactor to use optional.Option for issue index search option (#29739)
Browse files Browse the repository at this point in the history 
Signed-off-by: 6543 <6543@obermui.de>
  • Loading branch information
@6543
6543 authored Mar 13, 2024
1 parent 67e9f0d commit 7fd0a5b
Show file tree
Hide file tree
Showing 12 changed files with 178 additions and 225 deletions.
12 changes: 7 additions & 5 deletions modules/indexer/internal/bleve/query.go
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 7fd0a5b

Please sign in to comment.