Elasticsearch-net: NEST 6.0 GA: InferMappingFor and ConcreteTypeSelector gone

[tdoman]

In this PR, can now map clr types dynamically on connection settings including an…, I got an InferMappingFor() API that takes two parameters. Namely,

public TConnectionSettings InferMappingFor(Type documentType, Func<ClrTypeMappingDescriptor, IClrTypeMapping> selector)

It was there through RC1 but in the GA, it's gone. Where'd it go? Do I have to go back to the workaround described here: ConnectionSettings.MapDefaultTypeIndices in NEST 6.x?

[tdoman]

On a related note, we have also been using ConcreteTypeSelector on SearchDescriptor through RC1. Perhaps that's unneeded if we set up the inferred stuff properly? However, ConcreteTypeSelector is now also gone ... please advise.

[tdoman]

Just thought I'd try without the ConcreteTypeSelector and sure enough, apparently that's also required cuz now I get JObjects back and I'm using the workaround for inferred mappings listed above.

[tdoman]

I've played around a little w/ what I might be able to do on my own re: ConcreteTypeSelector but frankly, based on the fact that the PR I referred to above and is mentioned as a feature (#2934) in the release notes but is actually missing from the GA, I'm wondering if this also is just some oversight. Regardless, I was also surprised at losing ConcreteTypeSelector between RC1 and GA and other breaking changes between those two releases.

Anyway, I thought I might try this:

var hits = result.Hits.Select(x =>
{
   var indexType = ((JObject)x.Source).ToObject(typeIndex[x.Type].Type);
   ...

but of course, the JSON serializer is internal and I don't have access to Nest.Json.Linq so that didn't work. So, I figured I could resort to the following but this makes me sad on the inside. :)

var hits = result.Hits.Select(x =>
{
   var indexType = JsonConvert.DeserializeObject(x.Source.ToString(), typeIndex[x.Type].Type) as MyBaseType;
   ...

I don't have to resort to this, do I? We've gone through our other development environments w/ the ES 6.x code changes using RC1 and we were ready to go to production next week w/ the GA. As it stands, we'll be obliged to go w/ RC1 if I can't work around this issue.

[russcam]

Hey @tdoman

InferMappingFor() API that takes two parameters. Namely,

InferMappingFor() was renamed to DefaultMappingFor() in the GA release, to align the name with DefaultIndex(), DefaultTypeName, etc.

we have also been using ConcreteTypeSelector on SearchDescriptor through RC1.

ConcreteTypeSelector is gone in the GA release. Whilst a really nice feature to be able to return covariant results, with the _soft_ removal of types in Elasticsearch 6.0 and their hard removal scheduled for Elasticsearch 7.0, the feature won't be able to work properly, since it uses the "_type" hit metadata value to ascertain what the JSON object in the _source of the hit should be deserialized into. It's possible to perform something similar to the function of ConcreteTypeSelector using the NEST.JsonNetSerializer package in conjunction with TypeNameHandling.All if needing to search across multiple indices. Take a look at the custom serialization documentation for how to hook it up.

Let me know if you you have further questions, or if the documentation is not clear.

[tdoman]

Hi @russcam, thanks for the explanation. Understood, indeed we had already changed those of our index mappings where we had multiple types having consulted Removal of mapping types. However, since we had followed that requirement and it only seemed to essentially deprecate types, I anticipated no breaking change re: ConcreteTypeSelector until ES\NEST 7.x. As you said, it was a really nice feature and, even now, the hits still contain a .Type property so removing support in NEST 6.x seems premature.
That said, I've only lightly perused the documentation you provided and, at first glance, it appears to be very heavy handed for the purpose of merely emulating ConcreteTypeSelector. As I said, we had anticipated perhaps needing to go to production w/ RC1 which, based on our testing, we were comfortable with for another sprint. However, that was for the contingency that NEST 6.0 might not have gone GA. We weren't prepared for this breaking change and couldn't have anticipated it. I'll review the documentation more deeply as I am able and see if I can clarify w/ you here if it's indeed more straightforward than it looks.
One more thought, even when types go away, the need to search across multiple indices to return covariant results won't go away. Are you saying a custom serializer will be the only way to support this going forward? In other words, still seems like a very nice feature going forward.

[tdoman]

@russcam In reading further into the documentation you provided and also consulting Modifying the default serializer, I started to wonder if it was just as straightforward as modifying the default sourceSerializer w/ TypeNameHandling.All rather than, as I thought you might be saying, taking over all the (de)serialization myself. Simply, like so:

var connectionSettings = new ConnectionSettings(pool,
   sourceSerializer: (b, s) => new JsonNetSerializer(b, s,
      () => new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.All }));

However, it's obviously not that simple cuz I still get JObjects back in the hit results Source. I was initially assuming you were indicating that I'd have to derive and override the deserializer which was what seemed heavy-handed, especially when I'd expect I'm not the only one in need of ConcreteTypeSelector type behavior. Can you provider further guidance on what I'm missing?

[Mpdreamz]

What's the T that you use in Search<> @tdoman? If its object i'm 90% sure JSON.NET always gives you back a JObject or in this case our internalized Nest.JsonObject.

TypeNameHandling.All also assumes there is a $type property inside the serialized object in _source if you only added that for searches it won't do anything.

You should then be able to do .Search<MyBaseType>() and rely on JSON.NET handling the covariance for you.

Can you show me what your ConcreteTypeHandler looked like?

[tdoman]

@Mpdreamz you are exactly correct, I've been using Search<object> and that totally makes sense that JSON.NET could simply be giving me back a JObject in that case (in the debugger I'm pretty sure I was seeing Newtonsoft JObjects BTW).
My ConcreteTypeSelector looked like this:

.ConcreteTypeSelector((d, h) => _typeIndex[h.Type].Type)

where _typeIndex is a dictionary where I keep all kinds of information about the searches I want to do (ie. search fields, aggregations, etc.) including the POCO type. So, since the hits have the ES mapping type, in the delegate I can use my dictionary to look up my POCO type.
Now, based on what you were saying above, I've been playing around w/ what we have implemented to see if I can move to a .Search<MyBaseType> approach but I'll have to keep playing w/ it cuz it has become more than a little convoluted. I have a covariant interface that I've defined for use w/ disparate searches in a multi-search scenario. It requires each implementation to know how to make itself into a SearchDescriptor and turn an ISearchResponse into our models. As such, it's a bit of an "inception" of covariance but something I didn't have to deal with when I had a ConcreteTypeSelector. For now, I'm struggling to get parameterized typing throughout my abstraction and into a .Search<T> scenario.
What would be great, in the short term, would be a NEST 6.0.1 where ConcreteTypeSelector lives on in deprecation until ES\NEST 7.x where, when we're ready to upgrade, we can choose to work through types going away whilst implementing our own (if we must) covariant approach to multi-index searches.

[Mpdreamz]

They will be Newtonsoft JOBjects if you use Nest.JsonNetSerializer :)

Another option is to use: .Search<ILazyDocument>() you can then inspect the hits and call hit.Source.As<T>() or hit.Source.As(Type) in lieu of ConcreteTypeSelector. Hope this unblocks you for the time being.

Another reason for getting rid of ConceteTypeSelector and built in covariance is that internally it dictated the necessity for stateful serializers which introduced a very complex code path as we need to piggy back state into a contractresolver which brought in all sorts of challenges as sharing the contract resolver is key to performance so newing one for every stateful search is bad (something we mitigated by doing additional caching based on ConnectionSettings inside our contract resolver.

Having a completely simplified serialization flow internally opens us up to work on OOTB performance and allocation during the course of 6.x. Reintroducing ConcreteTypeConverter is something we'd like to avoid at all costs.

[russcam]

How do I fix issues?? How do I put in details

@ghavaun If you have an issue, please open a new issue and follow the instructions to describe what the issue is, how to reproduce it, etc. Thanks

[tdoman]

@Mpdreamz thank you very much! I got a chance to try out your suggestion a couple of days ago but hadn't had a chance to respond. That appears to be working as described and is a great approach for our current implementation and solid replacement for ConcreteTypeSelector. In addition, since I'm using the NEST.JsonNetSerializer, IIUC, I can also go back to the JsonProperty attribute for my POCO field mapping changes instead of PropertyName. As always, I appreciate your support. I'll close this issue for now, integrate this code change into our next sprint and re-open if I run into anything related to this but it seems to be working exactly as expected, thanks again.

[tdoman]

@Mpdreamz One follow up question I forgot. Once types are removed in ES 7, I think we'll need the alias name instead of the index name in the hits, right?. With the approach you suggested, I'm still keying off the Type property in the hits to check my dictionary for what POCO type to use. Currently, I see the index name there but nothing I would figure I could key upon for index to POCO type mapping.

[russcam]

IIUC, I can also go back to the JsonProperty attribute for my POCO field mapping changes instead of PropertyName

That's correct, since now Json.NET is responsible for serialization of your documents.

Currently, I see the index name there but nothing I would figure I could key upon for index to POCO type mapping

We've been discussing how best to handle this scenario. One thought is to have an index pattern -> CLR type mapping, but still need to let it bake.

[tdoman]

@russcam Cool, thanks Russ. I'm not sure how an index pattern would work for us since every index has a distinct type mapping but I'll be interested to hear what you guys come up with.