Skip to content

fallback to a sane default when edm parameters and clr parameters disagree #478

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 1 commit into from
Closed

fallback to a sane default when edm parameters and clr parameters disagree #478

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 10 additions & 6 deletions src/Common.OData.ApiExplorer/AspNet.OData/Routing/ODataRouteBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,14 @@ void AppendParametersFromConvention( StringBuilder builder, IEdmOperation operat
var actionParameters = Context.ParameterDescriptions.ToDictionary( p => p.Name, StringComparer.OrdinalIgnoreCase );
var parameter = parameters.Current;
var name = parameter.Name;
actionParameters.TryGetValue( name, out var mappedParameter );
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm still not sure I 100% grock the full use case, but I don't see anything glaring where this change is specific to your scenario. It appears to be a rational fallback, even if it shouldn't happen.

Let's simply change the entire setup to:

                var routeParameterName = name;
                
                if ( actionParameters.TryGetValue( name, out var mappedParameter ) )
                {
#if WEBAPI
                    routeParameterName = mappedParameter.ParameterDescriptor.ParameterName;
#elif API_EXPLORER
                    routeParameterName = mappedParameter.ParameterDescriptor.Name;
#else
                    routeParameterName = mappedParameter.Name;
#endif
                }

#if WEBAPI
var routeParameterName = actionParameters[name].ParameterDescriptor.ParameterName;
var routeParameterName = mappedParameter?.ParameterDescriptor?.ParameterName ?? name;
#elif API_EXPLORER
var routeParameterName = actionParameters[name].ParameterDescriptor.Name;

var routeParameterName = mappedParameter?.ParameterDescriptor?.Name ?? name;
#else
var routeParameterName = actionParameters[name].Name;
var routeParameterName = mappedParameter?.Name ?? name;
#endif

builder.Append( '(' );
Expand All @@ -239,12 +241,14 @@ void AppendParametersFromConvention( StringBuilder builder, IEdmOperation operat
{
parameter = parameters.Current;
name = parameter.Name;
actionParameters.TryGetValue( name, out mappedParameter );
Copy link
Collaborator

Choose a reason for hiding this comment

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

The change down here should obviously be the same.

#if WEBAPI
routeParameterName = actionParameters[name].ParameterDescriptor.ParameterName;
routeParameterName = mappedParameter?.ParameterDescriptor?.ParameterName ?? name;
#elif API_EXPLORER
routeParameterName = actionParameters[name].ParameterDescriptor.Name;

routeParameterName = mappedParameter?.ParameterDescriptor?.Name ?? name;
#else
routeParameterName = actionParameters[name].Name;
routeParameterName = mappedParameter?.Name ?? name;
#endif
builder.Append( ',' );
builder.Append( name );
Expand Down