-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
CSHARP-5442: Atlas Search doesn't respect the correct serializer #1583
base: main
Are you sure you want to change the base?
Changes from all commits
40d8bd9
65567ee
572ed5c
1493813
7f44c62
8dfa53f
a5ba406
78490cd
845c7eb
e283312
dd89854
7c7eafe
2f1af5f
f400017
5323c79
b776914
1cb7876
e480108
c27b824
5d47e61
fdf5846
bed20d9
60c4222
0fe3915
269f0dc
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 |
---|---|---|
|
@@ -17,9 +17,12 @@ | |
using System.Collections.Generic; | ||
using System.Linq; | ||
using MongoDB.Bson; | ||
using MongoDB.Bson.IO; | ||
using MongoDB.Bson.Serialization; | ||
using MongoDB.Bson.Serialization.Serializers; | ||
using MongoDB.Driver.Core.Misc; | ||
using MongoDB.Driver.GeoJsonObjectModel; | ||
using MongoDB.Driver.Linq.Linq3Implementation.Misc; | ||
|
||
namespace MongoDB.Driver.Search | ||
{ | ||
|
@@ -118,38 +121,35 @@ private protected override BsonDocument RenderArguments(RenderArgs<TDocument> ar | |
|
||
internal sealed class EqualsSearchDefinition<TDocument, TField> : OperatorSearchDefinition<TDocument> | ||
{ | ||
private readonly BsonValue _value; | ||
private readonly TField _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. Rename 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.
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.
In the case of an array 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. Did you remove the code you had earlier for |
||
|
||
public EqualsSearchDefinition(FieldDefinition<TDocument> path, TField value, SearchScoreDefinition<TDocument> score) | ||
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. It is unfortunate that This is not type safe because it allows mismatches between the field type and the value type that are not caught at compile time. In an earlier commit you had changed this but now it's removed. Can you explain why? 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 tried to uniform the code between the If this was a major version, I'd suggest to modify I think this is something that would be worth investigating for next major in order to improve the API, but I don't think we can do much at the moment without making breaking changes. (We also have |
||
: base(OperatorType.Equals, path, score) | ||
{ | ||
_value = ToBsonValue(value); | ||
_value = value; | ||
} | ||
|
||
private protected override BsonDocument RenderArguments(RenderArgs<TDocument> args) => | ||
new("value", _value); | ||
|
||
private static BsonValue ToBsonValue(TField value) => | ||
value switch | ||
private protected override BsonDocument RenderArguments(RenderArgs<TDocument> args) | ||
{ | ||
var searchPathDefinition = (SingleSearchPathDefinition<TDocument>)_path; | ||
var renderedField = searchPathDefinition.Field.Render(args); | ||
var valueSerializer = renderedField.FieldSerializer switch | ||
{ | ||
bool v => (BsonBoolean)v, | ||
sbyte v => (BsonInt32)v, | ||
byte v => (BsonInt32)v, | ||
short v => (BsonInt32)v, | ||
ushort v => (BsonInt32)v, | ||
int v => (BsonInt32)v, | ||
uint v => (BsonInt64)v, | ||
long v => (BsonInt64)v, | ||
float v => (BsonDouble)v, | ||
double v => (BsonDouble)v, | ||
DateTime v => (BsonDateTime)v, | ||
DateTimeOffset v => (BsonDateTime)v.UtcDateTime, | ||
ObjectId v => (BsonObjectId)v, | ||
Guid v => new BsonBinaryData(v, GuidRepresentation.Standard), | ||
string v => (BsonString)v, | ||
null => BsonNull.Value, | ||
_ => throw new InvalidCastException() | ||
null => BsonSerializer.LookupSerializer<TField>(), | ||
IBsonArraySerializer => ArraySerializerHelper.GetItemSerializer(renderedField.FieldSerializer), | ||
_ => renderedField.FieldSerializer | ||
}; | ||
|
||
var document = new BsonDocument(); | ||
using var bsonWriter = new BsonDocumentWriter(document); | ||
var context = BsonSerializationContext.CreateRoot(bsonWriter); | ||
bsonWriter.WriteStartDocument(); | ||
bsonWriter.WriteName("value"); | ||
valueSerializer.Serialize(context, _value); | ||
bsonWriter.WriteEndDocument(); | ||
|
||
return document; | ||
} | ||
} | ||
|
||
internal sealed class ExistsSearchDefinition<TDocument> : OperatorSearchDefinition<TDocument> | ||
|
@@ -225,7 +225,7 @@ private protected override BsonDocument RenderArguments(RenderArgs<TDocument> ar | |
|
||
internal sealed class InSearchDefinition<TDocument, TField> : OperatorSearchDefinition<TDocument> | ||
{ | ||
private readonly BsonArray _values; | ||
private readonly TField[] _values; | ||
|
||
public InSearchDefinition( | ||
SearchPathDefinition<TDocument> path, | ||
|
@@ -234,36 +234,38 @@ public InSearchDefinition( | |
: base(OperatorType.In, path, score) | ||
{ | ||
Ensure.IsNotNullOrEmpty(values, nameof(values)); | ||
var array = new BsonArray(values.Select(ToBsonValue)); | ||
|
||
var bsonType = array[0].GetType(); | ||
_values = Ensure.That(array, arr => arr.All(v => v.GetType() == bsonType), nameof(values), "All values must be of the same type."); | ||
_values = values.ToArray(); | ||
} | ||
|
||
private protected override BsonDocument RenderArguments(RenderArgs<TDocument> args) => | ||
new("value", _values); | ||
private protected override BsonDocument RenderArguments(RenderArgs<TDocument> args) | ||
{ | ||
IBsonSerializer valueSerializer; | ||
|
||
private static BsonValue ToBsonValue(TField value) => | ||
value switch | ||
if (_path is SingleSearchPathDefinition<TDocument> searchPathDefinition) | ||
{ | ||
bool v => (BsonBoolean)v, | ||
sbyte v => (BsonInt32)v, | ||
byte v => (BsonInt32)v, | ||
short v => (BsonInt32)v, | ||
ushort v => (BsonInt32)v, | ||
int v => (BsonInt32)v, | ||
uint v => (BsonInt64)v, | ||
long v => (BsonInt64)v, | ||
float v => (BsonDouble)v, | ||
double v => (BsonDouble)v, | ||
decimal v => (BsonDecimal128)v, | ||
DateTime v => (BsonDateTime)v, | ||
DateTimeOffset v => (BsonDateTime)v.UtcDateTime, | ||
string v => (BsonString)v, | ||
ObjectId v => (BsonObjectId)v, | ||
Guid v => new BsonBinaryData(v, GuidRepresentation.Standard), | ||
_ => throw new InvalidCastException() | ||
}; | ||
var renderedField = searchPathDefinition.Field.Render(args); | ||
valueSerializer = renderedField.FieldSerializer switch | ||
{ | ||
null => new ArraySerializer<TField>(BsonSerializer.LookupSerializer<TField>()), | ||
IBsonArraySerializer => renderedField.FieldSerializer, | ||
_ => new ArraySerializer<TField>((IBsonSerializer<TField>)renderedField.FieldSerializer) | ||
}; | ||
} | ||
else | ||
{ | ||
valueSerializer = new ArraySerializer<TField>(BsonSerializer.LookupSerializer<TField>()); | ||
} | ||
|
||
var document = new BsonDocument(); | ||
using var bsonWriter = new BsonDocumentWriter(document); | ||
var context = BsonSerializationContext.CreateRoot(bsonWriter); | ||
bsonWriter.WriteStartDocument(); | ||
bsonWriter.WriteName("value"); | ||
valueSerializer.Serialize(context, _values); | ||
bsonWriter.WriteEndDocument(); | ||
|
||
return document; | ||
} | ||
} | ||
|
||
internal sealed class MoreLikeThisSearchDefinition<TDocument, TLike> : OperatorSearchDefinition<TDocument> | ||
|
@@ -361,8 +363,6 @@ internal sealed class RangeSearchDefinition<TDocument, TField> : OperatorSearchD | |
where TField : struct, IComparable<TField> | ||
{ | ||
private readonly SearchRange<TField> _range; | ||
private readonly BsonValue _min; | ||
private readonly BsonValue _max; | ||
|
||
public RangeSearchDefinition( | ||
SearchPathDefinition<TDocument> path, | ||
|
@@ -371,34 +371,37 @@ public RangeSearchDefinition( | |
: base(OperatorType.Range, path, score) | ||
{ | ||
_range = range; | ||
_min = ToBsonValue(_range.Min); | ||
_max = ToBsonValue(_range.Max); | ||
} | ||
|
||
private protected override BsonDocument RenderArguments(RenderArgs<TDocument> args) => | ||
new() | ||
private protected override BsonDocument RenderArguments(RenderArgs<TDocument> args) | ||
{ | ||
var searchPathDefinition = (SingleSearchPathDefinition<TDocument>)_path; | ||
var renderedField = searchPathDefinition.Field.Render(args); | ||
var valueSerializer = renderedField.FieldSerializer switch | ||
{ | ||
{ _range.IsMinInclusive ? "gte" : "gt", _min, _min != null }, | ||
{ _range.IsMaxInclusive ? "lte" : "lt", _max, _max != null }, | ||
null => BsonSerializer.LookupSerializer<TField>(), | ||
IBsonArraySerializer => ArraySerializerHelper.GetItemSerializer(renderedField.FieldSerializer), | ||
_ => renderedField.FieldSerializer | ||
}; | ||
|
||
private static BsonValue ToBsonValue(TField? value) => | ||
value switch | ||
var document = new BsonDocument(); | ||
using var bsonWriter = new BsonDocumentWriter(document); | ||
var context = BsonSerializationContext.CreateRoot(bsonWriter); | ||
bsonWriter.WriteStartDocument(); | ||
if (_range.Min is not null) | ||
{ | ||
sbyte v => (BsonInt32)v, | ||
byte v => (BsonInt32)v, | ||
short v => (BsonInt32)v, | ||
ushort v => (BsonInt32)v, | ||
int v => (BsonInt32)v, | ||
uint v => (BsonInt64)v, | ||
long v => (BsonInt64)v, | ||
float v => (BsonDouble)v, | ||
double v => (BsonDouble)v, | ||
DateTime v => (BsonDateTime)v, | ||
DateTimeOffset v => (BsonDateTime)v.UtcDateTime, | ||
null => null, | ||
_ => throw new InvalidCastException() | ||
}; | ||
bsonWriter.WriteName(_range.IsMinInclusive? "gte" : "gt"); | ||
valueSerializer.Serialize(context, _range.Min); | ||
} | ||
if (_range.Max is not null) | ||
{ | ||
bsonWriter.WriteName(_range.IsMaxInclusive? "lte" : "lt"); | ||
valueSerializer.Serialize(context, _range.Max); | ||
} | ||
bsonWriter.WriteEndDocument(); | ||
|
||
return document; | ||
} | ||
} | ||
|
||
internal sealed class RegexSearchDefinition<TDocument> : OperatorSearchDefinition<TDocument> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,9 +123,9 @@ private protected enum OperatorType | |
QueryString, | ||
Range, | ||
Regex, | ||
Search, | ||
Search, //TODO This is never used, should we remove them? | ||
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. It looks like these two can be safely removed, should we do it? |
||
Span, | ||
Term, | ||
Term, //TODO This is never used, should we remove them? | ||
Text, | ||
Wildcard | ||
} | ||
|
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.
Should we remove this?