Repository navigation
Fix null reference error when matching binding type - #1700
Conversation
|
rebase & squash |
fix null ref error for same name functions Update ModelBuildingHelpers.cs fix typos Update SelectPathSegmentTokenBinder.cs Update ODataUriResolver.cs Revert filtering bound and unbound operations.
86ad1d5 to
d2b0c5d
Compare
|
|
||
| var function2 = new EdmFunction("Test", "Function", new EdmEntityTypeReference(vegetableType, true), false /*isBound*/, null /*entitySetPath*/, false); | ||
| function2.AddParameter("p1", new EdmEntityTypeReference(vegetableType, true)); | ||
| function2.AddParameter("p2", EdmCoreModel.Instance.GetInt32(false)); |
There was a problem hiding this comment.
Does this pass model validation? It should not -- you shouldn't be able to have two overloads of the same unbound function with the same parameter names.
There was a problem hiding this comment.
@mikepizzo Yes. I will follow up on that. In the meanwhile, it is a valid scenario for action overloads which differ by the binding type and there the parameter name does not even matter. Hypothetically, can the parameter name be the same but differ in type for a bound action.
To be on the safe side, I changed the above test to be a valid scenario with a different name. I will follow up with the behavior in model validation separately.
|
I'm okay with the changes as being more protective, but the added test case seems dubious; by spec, we should not allow multiple overloads of the same nonbinding function name with the same parameter names. I assume this is just testing the logic for unbound functions and not a real life scenario, but would also be interested if calling Validate() on the model succeeded (if so, we should file a separate issue to add this check to model validation). |
3 participants
Issues
This pull request fixes issue reported by Microsoft Graph
Description
This is an experiment. The nuget package from this build will be used to verify if the problem stops from happening.