Adding a new public constructor was a breaking change so I added a new static factory method instead.

Note: a new public constructor resulted in compile time ambiguity when called with null as the first argument.

One advantage of using Single here is that whenever LINQ2 calls this property it will throw an exception if the member is grouped under a nested path. That's far better than silently doing the wrong thing.

Agree with the fact that we want to throw here. But I think we have to throw something more readable rather that just "Sequence contains more then one element".

Fair enough. We're at POC stage right now.

We never call this method.

Should we delete it? Technically that's a breaking change, but the method no longer works reliably anyway since the server now allows . in element names.

Given that it is a breaking change, marking it as [Obsolete] is the right way forward IMHO.

This method was "broken" long before this PR.

We simply didn't realize it because we never call it.

Would it be OK to just delete it?

While I agree that the inclusion of . in element names breaks this method, that is not a mainline use case. This method works for the common case of element names without .. We can make the breaking change in 3.0.0, but not in a 2.X release IMHO.

I'm not proposing we make any changes to this method.

I'm merely pointing out that it's already broken, and that we don't use it.

Although I do wish we could just delete it.

This is an internal class so this is not a publicly breaking change.

This is a relatively simple change. We just need to call GetField more than once when there is a path.

If the path list has count == 1 this code is equivalent to the previous code.

Same as above but this time for filters.

I'm surprised I only found TWO places that needed to be changed. Hope I didn't miss any.

What about:
SelectMethodToFilterFieldTranslator line 47

Thanks. I missed that one.

I think we can ignore this one for now.

If it ever gets used when a path is involved the call to ElementName will throw an exception and we can deal with it then.

Note that OrderId and ProductId are grouped under the _id element.

Note that the use of $_id.OrderId instead of $OrderId.

This is for testing.

We would implement similar code in the EntitySerializer.

Oops this method is obsolete. Ignore it. It was when I was experimenting with a new interface instead.

I would suggest to have both: _elementPath and _elementName. We can use _elementName for situation when there is a single element path. And switch to the _elementPath in that rare case when there are multiple. It could save some allocations.

I understand that there is an additional allocation.

I was prioritizing simplicity and correctness over trying to optimize away an allocation.

The logic is that an ElementName is the same thing as an ElementPath with a single entry.

What I didn't want to do was force every user of this class to first test whether ElementName or ElementPath should be used. At that point we might as well use the interface approach and test whether the interface is implemented.

Do you think we can eliminate the allocation without complicating how this class can be consumed?

I guess a related question is: do we need a variable length path at all?

Is it sufficient to only support a single level of nesting?

I've pushed a commit that has mutually exclusive _elementName and _elementPath to reduce allocations.

Note the changes in the users of this class that now have to check whether ElementPath is null or not and only use ElementName when there is no ElementPath.

Agree with the fact that we want to throw here. But I think we have to throw something more readable rather that just "Sequence contains more then one element".

What about:
SelectMethodToFilterFieldTranslator line 47

Given this is a collection of paths, shouldn't this be ElementPaths?

I don't think so.

There is only ONE "path". The one path is composed of a series of "element names".

So path is not plural.

Another way to say it: this is not a collection of paths. A path is a collection of element names.

OK.

Given that it is a breaking change, marking it as [Obsolete] is the right way forward IMHO.

So it is the responsibility of OrderItemsSerializer to configure the composite _id. If we agree on this path forward, it would be useful to allow specification of multiple [BsonId(Order=N)] attributes as well as BsonClassMap<T> configuration.

I wasn't expecting to add any support to BsonClassMapSerializer for this.

I was thinking this would only be used by the EF Core Provider.

However, now that you ask about it, if we want applications written using the C# driver to interoperate with data written by the EF Core Provider I suppose we have to add support for configuring that kind of serialization in the driver also.

I agree that the most common use case is for EF Core, but it would be surprising for EF Core to write data that the driver cannot interoperate with.

While I agree that the inclusion of . in element names breaks this method, that is not a mainline use case. This method works for the common case of element names without .. We can make the breaking change in 3.0.0, but not in a 2.X release IMHO.

OK.

I agree that the most common use case is for EF Core, but it would be surprising for EF Core to write data that the driver cannot interoperate with.