Skip to content

Atlas search POC #956

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Atlas search POC #956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5c80dab
- Search text operator
BorisDog Nov 18, 2022
1c22dcf
- PR comments
BorisDog Nov 23, 2022
8f90467
Apply suggestions from code review
BorisDog Nov 23, 2022
c711b90
- Search added to MongoQueryable
BorisDog Nov 24, 2022
26a604b
Merge branch 'search_poc' of https://github.com/BorisDog/mongo-csharp…
BorisDog Nov 24, 2022
93f30b8
- PR comments
BorisDog Dec 1, 2022
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
101 changes: 52 additions & 49 deletions src/MongoDB.Driver.Core/Core/Misc/Ensure.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,18 @@ public static T IsGreaterThanOrEqualTo<T>(T value, T comparand, string paramName
}

/// <summary>
/// Ensures that the value of a parameter is greater than or equal to zero.
/// Ensures that the value of a parameter is greater than a comparand.
/// </summary>
/// <typeparam name="T">Type type of the value.</typeparam>
/// <param name="value">The value of the parameter.</param>
/// <param name="comparand">The comparand.</param>
/// <param name="paramName">The name of the parameter.</param>
/// <returns>The value of the parameter.</returns>
public static int IsGreaterThanOrEqualToZero(int value, string paramName)
public static T IsGreaterThan<T>(T value, T comparand, string paramName) where T : IComparable<T>
{
if (value < 0)
if (value.CompareTo(comparand) <= 0)
{
var message = string.Format("Value is not greater than or equal to 0: {0}.", value);
var message = $"Value is not greater than {comparand}: {value}.";
throw new ArgumentOutOfRangeException(paramName, message);
}
return value;
Expand All @@ -104,79 +106,62 @@ public static int IsGreaterThanOrEqualToZero(int value, string paramName)
/// <param name="value">The value of the parameter.</param>
/// <param name="paramName">The name of the parameter.</param>
/// <returns>The value of the parameter.</returns>
public static long IsGreaterThanOrEqualToZero(long value, string paramName)
{
if (value < 0)
{
var message = string.Format("Value is not greater than or equal to 0: {0}.", value);
throw new ArgumentOutOfRangeException(paramName, message);
}
return value;
}
public static int IsGreaterThanOrEqualToZero(int value, string paramName) =>
IsGreaterThanOrEqualTo(value, 0, paramName);

/// <summary>
/// Ensures that the value of a parameter is greater than or equal to zero.
/// </summary>
/// <param name="value">The value of the parameter.</param>
/// <param name="paramName">The name of the parameter.</param>
/// <returns>The value of the parameter.</returns>
public static TimeSpan IsGreaterThanOrEqualToZero(TimeSpan value, string paramName)
{
if (value < TimeSpan.Zero)
{
var message = string.Format("Value is not greater than or equal to zero: {0}.", TimeSpanParser.ToString(value));
throw new ArgumentOutOfRangeException(paramName, message);
}
return value;
}
public static long IsGreaterThanOrEqualToZero(long value, string paramName) =>
IsGreaterThanOrEqualTo(value, 0, paramName);
Copy link
Contributor

@rstam rstam Nov 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refactoring changes the error message a tiny bit. Could be considered a breaking change.

Copy link
Contributor

@rstam rstam Nov 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same might apply to some of the other refactorings.

Do we really want one Ensure method calling another? Or should each Ensure method be complete by itself?

Copy link
Contributor Author

@BorisDog BorisDog Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think code reuse makes things simpler and cleaner, where appropriate.

Copy link
Contributor

@rstam rstam Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course! But it changes the error messages.

Copy link
Contributor Author

@BorisDog BorisDog Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a very minor change in this case.
We also need to change messages in IsNullOr* anyway at some point.


/// <summary>
/// Ensures that the value of a parameter is greater than or equal to zero.
/// </summary>
/// <param name="value">The value of the parameter.</param>
/// <param name="paramName">The name of the parameter.</param>
/// <returns>The value of the parameter.</returns>
public static TimeSpan IsGreaterThanOrEqualToZero(TimeSpan value, string paramName) =>
IsGreaterThanOrEqualTo(value, TimeSpan.Zero, paramName);

/// <summary>
/// Ensures that the value of a parameter is greater than zero.
/// </summary>
/// <param name="value">The value of the parameter.</param>
/// <param name="paramName">The name of the parameter.</param>
/// <returns>The value of the parameter.</returns>
public static int IsGreaterThanZero(int value, string paramName)
{
if (value <= 0)
{
var message = string.Format("Value is not greater than zero: {0}.", value);
throw new ArgumentOutOfRangeException(paramName, message);
}
return value;
}
public static int IsGreaterThanZero(int value, string paramName) =>
IsGreaterThan(value, 0, paramName);

/// <summary>
/// Ensures that the value of a parameter is greater than zero.
/// </summary>
/// <param name="value">The value of the parameter.</param>
/// <param name="paramName">The name of the parameter.</param>
/// <returns>The value of the parameter.</returns>
public static long IsGreaterThanZero(long value, string paramName)
{
if (value <= 0)
{
var message = string.Format("Value is not greater than zero: {0}.", value);
throw new ArgumentOutOfRangeException(paramName, message);
}
return value;
}
public static long IsGreaterThanZero(long value, string paramName) =>
IsGreaterThan(value, 0, paramName);

/// <summary>
/// Ensures that the value of a parameter is greater than zero.
/// </summary>
/// <param name="value">The value of the parameter.</param>
/// <param name="paramName">The name of the parameter.</param>
/// <returns>The value of the parameter.</returns>
public static TimeSpan IsGreaterThanZero(TimeSpan value, string paramName)
{
if (value <= TimeSpan.Zero)
{
var message = string.Format("Value is not greater than zero: {0}.", value);
throw new ArgumentOutOfRangeException(paramName, message);
}
return value;
}
public static double IsGreaterThanZero(double value, string paramName) =>
IsGreaterThan(value, 0, paramName);

/// <summary>
/// Ensures that the value of a parameter is greater than zero.
/// </summary>
/// <param name="value">The value of the parameter.</param>
/// <param name="paramName">The name of the parameter.</param>
/// <returns>The value of the parameter.</returns>
public static TimeSpan IsGreaterThanZero(TimeSpan value, string paramName) =>
IsGreaterThan(value, TimeSpan.Zero, paramName);

/// <summary>
/// Ensures that the value of a parameter is infinite or greater than or equal to zero.
Expand Down Expand Up @@ -298,6 +283,24 @@ public static T IsNull<T>(T value, string paramName) where T : class
return value;
}

/// <summary>
/// Ensures that the value of a parameter is null or is between a minimum and a maximum value.
/// </summary>
/// <typeparam name="T">Type type of the value.</typeparam>
/// <param name="value">The value of the parameter.</param>
/// <param name="min">The minimum value.</param>
/// <param name="max">The maximum value.</param>
/// <param name="paramName">The name of the parameter.</param>
/// <returns>The value of the parameter.</returns>
public static T? IsNullOrBetween<T>(T? value, T min, T max, string paramName) where T : struct, IComparable<T>
{
if (value != null)
{
IsBetween(value.Value, min, max, paramName);
}
return value;
}

/// <summary>
/// Ensures that the value of a parameter is null or greater than or equal to zero.
/// </summary>
Expand Down
17 changes: 14 additions & 3 deletions src/MongoDB.Driver/AggregateFluent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@
* limitations under the License.
*/

using MongoDB.Bson;
using MongoDB.Bson.Serialization;
using MongoDB.Driver.Core.Misc;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using MongoDB.Bson;
using MongoDB.Bson.Serialization;
using MongoDB.Driver.Core.Misc;
using MongoDB.Driver.Search;

namespace MongoDB.Driver
{
Expand Down Expand Up @@ -238,6 +239,16 @@ public override IAggregateFluent<TNewResult> ReplaceWith<TNewResult>(AggregateEx
return WithPipeline(_pipeline.ReplaceWith(newRoot));
}

public override IAggregateFluent<TResult> Search(
SearchDefinition<TResult> query,
HighlightOptions<TResult> highlight = null,
string indexName = null,
SearchCountOptions count = null,
bool returnStoredSource = false)
{
return WithPipeline(_pipeline.Search(query, highlight, indexName, count, returnStoredSource));
}

public override IAggregateFluent<BsonDocument> SetWindowFields<TWindowFields>(
AggregateExpressionDefinition<ISetWindowFieldsPartition<TResult>, TWindowFields> output)
{
Expand Down
12 changes: 12 additions & 0 deletions src/MongoDB.Driver/AggregateFluentBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using System.Threading.Tasks;
using MongoDB.Bson;
using MongoDB.Bson.Serialization;
using MongoDB.Driver.Search;

namespace MongoDB.Driver
{
Expand Down Expand Up @@ -215,6 +216,17 @@ public virtual IAggregateFluent<TNewResult> ReplaceWith<TNewResult>(AggregateExp
throw new NotImplementedException();
}

/// <inheritdoc />
public virtual IAggregateFluent<TResult> Search(
SearchDefinition<TResult> query,
HighlightOptions<TResult> highlight = null,
string indexName = null,
SearchCountOptions count = null,
bool returnStoredSource = false)
{
throw new NotImplementedException();
}

/// <inheritdoc />
public virtual IAggregateFluent<BsonDocument> SetWindowFields<TWindowFields>(
AggregateExpressionDefinition<ISetWindowFieldsPartition<TResult>, TWindowFields> output)
Expand Down
61 changes: 21 additions & 40 deletions src/MongoDB.Driver/Builders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
* limitations under the License.
*/

using MongoDB.Driver.Search;

Copy link
Contributor

@rstam rstam Nov 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of irrelevant refactoring in this file.

Copy link
Contributor Author

@BorisDog BorisDog Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good opportunity for minor refactoring. For example if staying with fields, readonly needs to be added anyway.

namespace MongoDB.Driver
{
/// <summary>
Expand All @@ -21,50 +23,29 @@ namespace MongoDB.Driver
/// <typeparam name="TDocument">The type of the document.</typeparam>
public static class Builders<TDocument>
{
private static FilterDefinitionBuilder<TDocument> __filter = new FilterDefinitionBuilder<TDocument>();
private static IndexKeysDefinitionBuilder<TDocument> __index = new IndexKeysDefinitionBuilder<TDocument>();
private static ProjectionDefinitionBuilder<TDocument> __projection = new ProjectionDefinitionBuilder<TDocument>();
private static SortDefinitionBuilder<TDocument> __sort = new SortDefinitionBuilder<TDocument>();
private static UpdateDefinitionBuilder<TDocument> __update = new UpdateDefinitionBuilder<TDocument>();
Copy link
Contributor

@rstam rstam Nov 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with removing private backing fields.

Copy link
Contributor Author

@BorisDog BorisDog Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a classic usage for auto properties.
The only purpose of this class is to expose those properties, so we can let it do just that.

Copy link
Contributor

@rstam rstam Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My counter argument is that most of our classes already have private fields using the _field or __staticField naming convention.

In many cases (such as when validating in setters) an explicit backing field is required.

Why not just standardize on explicit backing fields and that way we can always know by glancing at the top of a class what state the class maintains.

Otherwise we have to tediously search the source file to see if there are any autoproperties we didn't know about.

Copy link
Contributor Author

@BorisDog BorisDog Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the key here is finding a good balance between following the existing style and evolving our code base to use new language features and mainstream standards. We have already adopted some mainstream conventions like string interpolation, nameof, local methods and other, despite having most of the code written differently. I hope we continue to do so.
Autoproperties are a very good example of a "mainstream" and very loved feature by C# community (which continues to evolve), and I believe it is perceived as sort of "default" in C# dev process.

Like with fields, all properties (auto implemented or not) through the source file, they will always reside in same section, like fields. Given that, and the fact the major part of code is cleaned up, the "state" of the class appears more clear to me. Autoproperties indicate that this is just a simple data holders, no custom properties, no special relation between property and field that needs to be inspected.

For more complicated classes, where inspecting class state/functionality becomes is more complicated, and I find using VS built-in class/document explorers very useful.

/// <summary>Gets a <see cref="FilterDefinitionBuilder{TDocument}"/>.</summary>
public static FilterDefinitionBuilder<TDocument> Filter { get; } = new FilterDefinitionBuilder<TDocument>();

/// <summary>Gets an <see cref="IndexKeysDefinitionBuilder{TDocument}"/>.</summary>
public static IndexKeysDefinitionBuilder<TDocument> IndexKeys { get; } = new IndexKeysDefinitionBuilder<TDocument>();

/// <summary>Gets a <see cref="ProjectionDefinitionBuilder{TDocument}"/>.</summary>
public static ProjectionDefinitionBuilder<TDocument> Projection { get; } = new ProjectionDefinitionBuilder<TDocument>();

/// <summary>
/// Gets a <see cref="FilterDefinitionBuilder{TDocument}"/>.
/// </summary>
public static FilterDefinitionBuilder<TDocument> Filter
{
get { return __filter; }
}
/// <summary>Gets a <see cref="SortDefinitionBuilder{TDocument}"/>.</summary>
public static SortDefinitionBuilder<TDocument> Sort { get; } = new SortDefinitionBuilder<TDocument>();

/// <summary>
/// Gets an <see cref="IndexKeysDefinitionBuilder{TDocument}"/>.
/// </summary>
public static IndexKeysDefinitionBuilder<TDocument> IndexKeys
{
get { return __index; }
}
/// <summary>Gets an <see cref="UpdateDefinitionBuilder{TDocument}"/>.</summary>
public static UpdateDefinitionBuilder<TDocument> Update { get; } = new UpdateDefinitionBuilder<TDocument>();

/// <summary>
/// Gets a <see cref="ProjectionDefinitionBuilder{TDocument}"/>.
/// </summary>
public static ProjectionDefinitionBuilder<TDocument> Projection
{
get { return __projection; }
}
// Search builders
/// <summary>Gets a <see cref="PathDefinition{TDocument}"/>.</summary>
public static PathDefinitionBuilder<TDocument> Path { get; } = new PathDefinitionBuilder<TDocument>();

/// <summary>
/// Gets a <see cref="SortDefinitionBuilder{TDocument}"/>.
/// </summary>
public static SortDefinitionBuilder<TDocument> Sort
{
get { return __sort; }
}
/// <summary>Gets a <see cref="ScoreDefinitionBuilder{TDocument}"/>.</summary>
public static ScoreDefinitionBuilder<TDocument> Score { get; } = new ScoreDefinitionBuilder<TDocument>();

/// <summary>
/// Gets an <see cref="UpdateDefinitionBuilder{TDocument}"/>.
/// </summary>
public static UpdateDefinitionBuilder<TDocument> Update
{
get { return __update; }
}
/// <summary>Gets a <see cref="SearchDefinitionBuilder{TDocument}"/>.</summary>
public static SearchDefinitionBuilder<TDocument> Search { get; } = new SearchDefinitionBuilder<TDocument>();
}
}
20 changes: 20 additions & 0 deletions src/MongoDB.Driver/IAggregateFluent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using System.Threading.Tasks;
using MongoDB.Bson;
using MongoDB.Bson.Serialization;
using MongoDB.Driver.Search;

namespace MongoDB.Driver
{
Expand Down Expand Up @@ -353,6 +354,25 @@ IAggregateFluent<TNewResult> Lookup<TForeignDocument, TAsElement, TAs, TNewResul
IAggregateFluent<BsonDocument> SetWindowFields<TWindowFields>(
AggregateExpressionDefinition<ISetWindowFieldsPartition<TResult>, TWindowFields> output);

/// <summary>
/// Appends a $search stage to the pipeline.
/// </summary>
/// <param name="query">The search definition.</param>
/// <param name="highlight">The highlight options.</param>
/// <param name="indexName">The index name.</param>
/// <param name="count">The count options.</param>
/// <param name="returnStoredSource">
/// Flag that specifies whether to perform a full document lookup on the backend database
/// or return only stored source fields directly from Atlas Search.
/// </param>
/// <returns>The fluent aggregate interface.</returns>
IAggregateFluent<TResult> Search(
SearchDefinition<TResult> query,
HighlightOptions<TResult> highlight = null,
string indexName = null,
SearchCountOptions count = null,
bool returnStoredSource = false);

/// <summary>
/// Appends a $setWindowFields to the pipeline.
/// </summary>
Expand Down
28 changes: 28 additions & 0 deletions src/MongoDB.Driver/Linq/MongoQueryable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using System.Threading;
using System.Threading.Tasks;
using MongoDB.Bson.Serialization;
using MongoDB.Driver.Search;

namespace MongoDB.Driver.Linq
{
Expand Down Expand Up @@ -923,6 +924,33 @@ public static IMongoQueryable<TSource> Sample<TSource>(this IMongoQueryable<TSou
Expression.Constant(count)));
}

/// <summary>
/// Appends a $search stage to the LINQ pipeline
/// </summary>
/// <typeparam name="TSource">The type of the elements of <paramref name="source" />.</typeparam>
/// <param name="source">A sequence of values.</param>
/// <param name="searchDefinition">The search definition.</param>
/// <param name="highlight">The highlight options.</param>
/// <param name="indexName">The index name.</param>
/// <param name="count">The count options.</param>
/// <param name="returnStoredSource">
/// Flag that specifies whether to perform a full document lookup on the backend database
/// or return only stored source fields directly from Atlas Search.
/// </param>
/// <returns>The fluent aggregate interface.</returns>
public static IMongoQueryable<TSource> Search<TSource>(
this IMongoQueryable<TSource> source,
SearchDefinition<TSource> searchDefinition,
HighlightOptions<TSource> highlight = null,
string indexName = null,
SearchCountOptions count = null,
bool returnStoredSource = false)
{
return AppendStage(
source,
PipelineStageDefinitionBuilder.Search(searchDefinition, highlight, indexName, count, returnStoredSource));
}

/// <summary>
/// Projects each element of a sequence into a new form by incorporating the
/// element's index.
Expand Down
5 changes: 3 additions & 2 deletions src/MongoDB.Driver/MongoUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
*/

using System;
using System.Runtime.InteropServices;
using System.Security;
using System.Security.Cryptography;
using System.Text;
using MongoDB.Bson;
Expand Down Expand Up @@ -61,5 +59,8 @@ public static string ToCamelCase(string value)
{
return value.Length == 0 ? "" : value.Substring(0, 1).ToLower() + value.Substring(1);
}

internal static string ToCamelCase<TEnum>(this TEnum @enum) where TEnum : Enum =>
ToCamelCase(@enum.ToString());
}
}
Loading