-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix(server): enable original key filter #57
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -27,64 +27,7 @@ func (c *Collection) Paginate(ctx context.Context, rawFilter any, s *usecasex.So | |||||
filter = And(rawFilter, "", pFilter) | ||||||
} | ||||||
|
||||||
sortKey := idKey | ||||||
sortOrder := 1 | ||||||
if s != nil && s.Key != "" { | ||||||
sortKey = s.Key | ||||||
if s.Reverted { | ||||||
sortOrder = -1 | ||||||
} | ||||||
} | ||||||
|
||||||
if p.Cursor != nil && p.Cursor.Last != nil { | ||||||
sortOrder *= -1 | ||||||
} | ||||||
|
||||||
sort := bson.D{{Key: sortKey, Value: sortOrder}} | ||||||
if sortKey != idKey { | ||||||
sort = append(sort, bson.E{Key: idKey, Value: sortOrder}) | ||||||
} | ||||||
|
||||||
findOpts := options.Find(). | ||||||
SetSort(sort). | ||||||
SetLimit(limit(*p)) | ||||||
|
||||||
if p.Offset != nil { | ||||||
findOpts.SetSkip(p.Offset.Offset) | ||||||
} | ||||||
|
||||||
cursor, err := c.collection.Find(ctx, filter, append([]*options.FindOptions{findOpts}, opts...)...) | ||||||
if err != nil { | ||||||
return nil, rerror.ErrInternalByWithContext(ctx, fmt.Errorf("failed to find: %w", err)) | ||||||
} | ||||||
defer func() { | ||||||
_ = cursor.Close(ctx) | ||||||
}() | ||||||
|
||||||
count, err := c.collection.CountDocuments(ctx, rawFilter) | ||||||
if err != nil { | ||||||
return nil, rerror.ErrInternalByWithContext(ctx, fmt.Errorf("failed to count: %w", err)) | ||||||
} | ||||||
|
||||||
items, startCursor, endCursor, hasMore, err := consume(ctx, cursor, limit(*p)) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
|
||||||
if p.Cursor != nil && p.Cursor.Last != nil { | ||||||
reverse(items) | ||||||
startCursor, endCursor = endCursor, startCursor | ||||||
} | ||||||
|
||||||
for _, item := range items { | ||||||
if err := consumer.Consume(item); err != nil { | ||||||
return nil, err | ||||||
} | ||||||
} | ||||||
|
||||||
hasNextPage, hasPreviousPage := pageInfo(p, hasMore) | ||||||
|
||||||
return usecasex.NewPageInfo(count, startCursor, endCursor, hasNextPage, hasPreviousPage), nil | ||||||
return c.paginate(ctx, rawFilter, s, p, filter, consumer) | ||||||
} | ||||||
|
||||||
func (c *Collection) PaginateAggregation(ctx context.Context, pipeline []any, s *usecasex.Sort, p *usecasex.Pagination, consumer Consumer, opts ...*options.AggregateOptions) (*usecasex.PageInfo, error) { | ||||||
|
@@ -327,3 +270,84 @@ func sortDirection(p usecasex.Pagination, s *usecasex.Sort) int { | |||||
} | ||||||
return 1 | ||||||
} | ||||||
|
||||||
func (c *Collection) PaginateProject(ctx context.Context, rawFilter any, s *usecasex.Sort, p *usecasex.Pagination, consumer Consumer, opts ...*options.FindOptions) (*usecasex.PageInfo, error) { | ||||||
if p == nil || (p.Cursor == nil && p.Offset == nil) { | ||||||
return nil, nil | ||||||
} | ||||||
|
||||||
pFilter, err := c.pageFilter(ctx, *p, s) | ||||||
if err != nil { | ||||||
return nil, rerror.ErrInternalByWithContext(ctx, err) | ||||||
} | ||||||
|
||||||
filter := rawFilter | ||||||
if pFilter != nil { | ||||||
filter = AddCondition(rawFilter, "", pFilter) | ||||||
} | ||||||
|
||||||
return c.paginate(ctx, rawFilter, s, p, filter, consumer) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential mismatch in document count due to In Consider updating the call to use -return c.paginate(ctx, rawFilter, s, p, filter, consumer)
+return c.paginate(ctx, filter, s, p, filter, consumer) 📝 Committable suggestion
Suggested change
|
||||||
|
||||||
} | ||||||
|
||||||
func (c *Collection) paginate(ctx context.Context, rawFilter any, s *usecasex.Sort, p *usecasex.Pagination, filter any, consumer Consumer) (*usecasex.PageInfo, error) { | ||||||
|
||||||
sortKey := idKey | ||||||
sortOrder := 1 | ||||||
if s != nil && s.Key != "" { | ||||||
sortKey = s.Key | ||||||
if s.Reverted { | ||||||
sortOrder = -1 | ||||||
} | ||||||
} | ||||||
|
||||||
if p.Cursor != nil && p.Cursor.Last != nil { | ||||||
sortOrder *= -1 | ||||||
} | ||||||
|
||||||
sort := bson.D{{Key: sortKey, Value: sortOrder}} | ||||||
if sortKey != idKey { | ||||||
sort = append(sort, bson.E{Key: idKey, Value: sortOrder}) | ||||||
} | ||||||
|
||||||
findOpts := options.Find(). | ||||||
SetSort(sort). | ||||||
SetLimit(limit(*p)) | ||||||
|
||||||
if p.Offset != nil { | ||||||
findOpts.SetSkip(p.Offset.Offset) | ||||||
} | ||||||
|
||||||
cursor, err := c.collection.Find(ctx, filter, append([]*options.FindOptions{findOpts}, opts...)...) | ||||||
if err != nil { | ||||||
return nil, rerror.ErrInternalByWithContext(ctx, fmt.Errorf("failed to find: %w", err)) | ||||||
} | ||||||
defer func() { | ||||||
_ = cursor.Close(ctx) | ||||||
}() | ||||||
|
||||||
count, err := c.collection.CountDocuments(ctx, rawFilter) | ||||||
if err != nil { | ||||||
return nil, rerror.ErrInternalByWithContext(ctx, fmt.Errorf("failed to count: %w", err)) | ||||||
} | ||||||
hexaforce marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
items, startCursor, endCursor, hasMore, err := consume(ctx, cursor, limit(*p)) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
|
||||||
if p.Cursor != nil && p.Cursor.Last != nil { | ||||||
reverse(items) | ||||||
startCursor, endCursor = endCursor, startCursor | ||||||
} | ||||||
|
||||||
for _, item := range items { | ||||||
if err := consumer.Consume(item); err != nil { | ||||||
return nil, err | ||||||
} | ||||||
} | ||||||
|
||||||
hasNextPage, hasPreviousPage := pageInfo(p, hasMore) | ||||||
|
||||||
return usecasex.NewPageInfo(count, startCursor, endCursor, hasNextPage, hasPreviousPage), nil | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent use of
rawFilter
vsfilter
when callingpaginate
In the
Paginate
method, you passrawFilter
as therawFilter
argument toc.paginate
, whilefilter
may include additional pagination conditions (pFilter
). This could lead to inconsistencies during document counting inpaginate
, asrawFilter
doesn't contain these additional conditions.To ensure consistency between the documents counted and those retrieved, consider passing
filter
instead ofrawFilter
:📝 Committable suggestion