-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-5672: Support sorting by value in PushEach operation #1748
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
base: main
Are you sure you want to change the base?
Changes from all 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 |
---|---|---|
|
@@ -130,6 +130,15 @@ public static SortDefinition<TDocument> MetaTextScore<TDocument>(this SortDefini | |
/// <typeparam name="TDocument">The type of the document.</typeparam> | ||
public sealed class SortDefinitionBuilder<TDocument> | ||
{ | ||
/// <summary> | ||
/// Creates a value ascending sort. | ||
/// </summary> | ||
/// <returns>An ascending sort.</returns> | ||
public SortDefinition<TDocument> Ascending() | ||
{ | ||
return new NoFieldDirectionalSortDefinition<TDocument>(SortDirection.Ascending); | ||
} | ||
|
||
/// <summary> | ||
/// Creates an ascending sort. | ||
/// </summary> | ||
|
@@ -170,6 +179,15 @@ public SortDefinition<TDocument> Combine(IEnumerable<SortDefinition<TDocument>> | |
return new CombinedSortDefinition<TDocument>(sorts); | ||
} | ||
|
||
/// <summary> | ||
/// Creates a value descending sort. | ||
/// </summary> | ||
/// <returns>A descending sort.</returns> | ||
public SortDefinition<TDocument> Descending() | ||
{ | ||
return new NoFieldDirectionalSortDefinition<TDocument>(SortDirection.Descending); | ||
} | ||
|
||
/// <summary> | ||
/// Creates a descending sort. | ||
/// </summary> | ||
|
@@ -232,6 +250,11 @@ internal sealed class CombinedSortDefinition<TDocument> : SortDefinition<TDocume | |
public CombinedSortDefinition(IEnumerable<SortDefinition<TDocument>> sorts) | ||
{ | ||
_sorts = Ensure.IsNotNull(sorts, nameof(sorts)).ToList(); | ||
|
||
if (_sorts.Any(sort => sort is NoFieldDirectionalSortDefinition<TDocument>)) | ||
{ | ||
throw new InvalidOperationException("Value-based sort cannot be combined with other sorts. When sorting by the entire element value, no other sorting criteria can be applied."); | ||
} | ||
} | ||
|
||
public override BsonDocument Render(RenderArgs<TDocument> args) | ||
|
@@ -288,4 +311,26 @@ public override BsonDocument Render(RenderArgs<TDocument> args) | |
return new BsonDocument(renderedField.FieldName, value); | ||
} | ||
} | ||
|
||
internal sealed class NoFieldDirectionalSortDefinition<TDocument> : SortDefinition<TDocument> | ||
{ | ||
private readonly SortDirection _direction; | ||
|
||
public NoFieldDirectionalSortDefinition(SortDirection direction) | ||
{ | ||
_direction = direction; | ||
} | ||
|
||
public override BsonDocument Render(RenderArgs<TDocument> args) | ||
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. Do we want a 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. We currently have no use for that property, so I decided to omit it for now to avoid introducing unused code. |
||
{ | ||
BsonValue value = _direction switch | ||
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. We do the |
||
{ | ||
SortDirection.Ascending => 1, | ||
SortDirection.Descending => -1, | ||
_ => throw new InvalidOperationException("Unknown value for " + typeof(SortDirection) + ".") | ||
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. I would have done this:
but I suppose then we might want to change |
||
}; | ||
|
||
return new BsonDocument("direction", value); | ||
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. When we use "magic" values like this we usually enclose them in angle brackets to make it visually apparent that something is special about this value:
We only have a field name here because 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. So what happens in other cases (which are not We could add 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. Nice, I like this! I thought of something similar but thought adding something to SortDefinition would be a breaking change but I suppose adding an internal virtual method is not? Learning some library maintainer tricks :) |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1685,7 +1685,13 @@ public override BsonValue Render(RenderArgs<TDocument> args) | |
|
||
if (_sort != null) | ||
{ | ||
document["$push"][renderedField.FieldName]["$sort"] = _sort.Render(args.WithNewDocumentType((IBsonSerializer<TItem>)itemSerializer)); | ||
var newArgs = args.WithNewDocumentType((IBsonSerializer<TItem>)itemSerializer); | ||
|
||
var renderedSort = _sort is NoFieldDirectionalSortDefinition<TItem> | ||
? _sort.Render(newArgs)["direction"] | ||
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.
Note the added angle brackets. |
||
: _sort.Render(newArgs); | ||
|
||
document["$push"][renderedField.FieldName]["$sort"] = renderedSort; | ||
} | ||
|
||
return document; | ||
|
Uh oh!
There was an error while loading. Please reload this page.