-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Feature/auto routing #3009
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
Feature/auto routing #3009
Conversation
…e it easy to grab to proper routing value given a document that has a join field
… does it do when called twice?
… the infered route for the instance of T
…mechanism to add better description for the high level client generated code. Add abbility to provide Route property for inference on ConnectionSettings
…ly properties that were not needed, bulk operations now infer routing
…ery string parameter
…g true on the query string due to codegen bug'
| @Raw(string.Join("\r\n\t\t", desc.Select(d=> "/// " + d))) | ||
| ///</summary></text> | ||
| }</text><text>@if (!string.IsNullOrWhiteSpace(kv.Value.Obsolete)) | ||
| {<text> [Obsolete("Scheduled to be removed in 7.0, @kv.Value.Obsolete")]</text>} |
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.
👍
| @foreach (CsharpMethod method in Model.CsharpMethodsWithQueryStringInfo) | ||
| { | ||
| if (!method.Url.Params.Any(p => p.Key.Contains("field") || p.Key.Contains("source_"))) | ||
| if (!method.Url.Params.Any(p => p.Key.Contains("field") || p.Key.Contains("source_") || p.Key.Contains("routing"))) |
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.
These chained conditions always scare me as I have to read to work out what they mean.
Much prefer:
var nameOfCompositeCondition = ............................;
if (nameOfCompositeCondition) { ............ }
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.
👍
| }</text><text>@if(!string.IsNullOrWhiteSpace(kv.Value.DeprecatedInFavorOf)) | ||
| { | ||
| <text>[Obsolete("Scheduled to be removed in 5.0, use @kv.Value.DeprecatedInFavorOf instead")]</text> | ||
| <text>[Obsolete("Scheduled to be removed in 7.0, use @kv.Value.DeprecatedInFavorOf instead")]</text> |
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.
👍
codebrain
left a comment
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.
Some changes.
This review has taken quite a bit of time!
| } | ||
|
|
||
| /** | ||
| * ==== Inferring from a type |
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.
The real power of the Routingis in the inference rules (the default inferred routing for an object will be null)
| /** | ||
| * ==== JoinField | ||
| * | ||
| * If your POCO has a `JoinField` property however NEST will automatically infer the parentid as the routing value. |
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.
If your class has a property of type JoinField, NEST will automatically infer the parentid as the routing value.
| Expect("8080").WhenInferringRoutingOn(dto); | ||
|
|
||
| /** | ||
| * here we link this instance as the root (parent) of the relation. Nest is smart enough to infer that the default routing |
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.
Here we link this instance as the root (parent) of the relation. NEST infers that the default routing for this instance should be the Id of the document itself.
| /** | ||
| * ==== Precedence of ConnectionSettings | ||
| * | ||
| * The routing property configured on `ConnectionSettings` however always takes precedence. |
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.
remove 'however'
| } | ||
| [U] public void DuplicateJoinField() | ||
| { | ||
| /** a property with more than one JoinField is not allowed */ |
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.
A class cannot contain more than one property of type JoinField, an exception is thrown in this case...
| Action resolve = () => Expect("8080").WhenInferringRoutingOn(dto); | ||
| resolve.ShouldThrow<ArgumentException>().WithMessage("BadDTO has more than one JoinField property"); | ||
|
|
||
| /** unless you configure NEST to look elsewhere */ |
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.
... unless you configure the ConnectionSettings to use an alternate property:
| * ==== Default mapping for String properties | ||
| * ==== Parent And Child mapping | ||
| * | ||
| * In the following example we setup our client and give our types prefered index and type names. What's new is that we can |
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.
Delete 'What's new' and use 'In NEST 6.x'
| */ | ||
| * ==== Indexing parents or children | ||
| * | ||
| * Now that we have our join field mapping set up on the index we can proceed to index parents and children documents. |
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.
use phrase 'parent and child documents'
This PR strongly types the
routingparameter toRoutingand introduces a new feature called auto routing.This feature will automatically append
routing=keyif we see the Poco is using aJoinFieldin which case we will send theparent idprovided there. If the join field itself is the root and only defines the root relation name NEST will send the id of the root object. This plays nicely when_routingis required on the mapping. This will hopefully aid people as they move to single types and the new way to do parent and child mappings. Also finished the documentation section for parent and child mappings 6.x style so that we can link to it from the 6.0 GA release blog post.You can also configure a default routing lookup property on
ConnectionSettingsthroughInferMappingFor<>(). This takes precedence over NEST looking for aJoinFieldproperty.These defaults kick in for request and objects (LikeDocument. BulkOperation, MultiTermVectorOperation, PercolateQuery) that have a routing parameter AND a
.NETdocument to infer on. For requests this is automated from the code generator.And lastly an explicitly set
Routing(x)orRouting = xby the user takes precedence over all, and ifx == nullit will nullify the routing.Important to note: For POCO's that have neither a JoinField or a mapping set up for routing on the
ConnectionSettingstheroutingparameter is not send automatically so existing code should not be affected.See docs:
https://github.com/elastic/elasticsearch-net/blob/01b7731dab069e8967d3b3baccda0efa092f800d/src/Tests/ClientConcepts/HighLevel/Mapping/ParentChildJoins.doc.cs
https://github.com/elastic/elasticsearch-net/blob/01b7731dab069e8967d3b3baccda0efa092f800d/src/Tests/ClientConcepts/HighLevel/Inference/RoutingInference.doc.cs