We have moved to GitHub Issues
Created by Claus Jensen 24 Jan 2017, 19:01:21 Updated by Martin Griffiths 02 Feb 2017, 15:43:24Tags: Unscheduled Regression
Relates to: U4-9436
Relates to: U4-9312
Won't work as there's now multiple actions matching (it can't differentiate between the int/string type in parameters)
@Shandem any ideas?
Is there a way to fix this without completely reverting it?
@Alex.Lindgren yep .. changing the name of the new method so it doesn't clash with the old one - and then keep the original signature for the old one.
As for a fix to anyone using the controller, whose code has broken in 7.5.8, the fix is easy as it's just making sure you do
.ToString() on the
int you're passing to the action.
That, unfortunately won't fix anything though.. cat's out of the bag now: people have already changed their code to call the method with
ToString() now so if we remove that method again it will be another breaking change :-)
--This should do the trick: https://github.com/umbraco/Umbraco-CMS/pull/1706--
@sebastiaan Since you're doing exactly the same as I was in my original PR, I can assure you it won't ;)
You will have duplicated controller actions that it can't figure out how to resolve due to the param being sent being something like
-1. It can't figure out whether to parse it as an
int or a
string when it is in a querystring.
Fix in https://github.com/umbraco/Umbraco-CMS/pull/1703 is still the only way to solve it.
I understand now what the problem was, WebApi calls didn't like the ambiguous method.
Since you now added an additional method, they don't collide any more and that's great! I've just added an additional method to help people who have already updated their code to work around the original issue of the method signature change. So now if you already did
.ToString() in your code, it will just work. It's not pretty but at least we're not breaking the same sites for a second time:
Realized that I made a new method that was only internally used, so made it private! https://github.com/umbraco/Umbraco-CMS/pull/1703/commits/c857d822d82d84e95a181d347e28ebe29121ffcc
@sebastiaan Your latest commits won't work.
You didn't fix the
MediaTypeController.GetAllowedChildren - only the
This fix won't work, since anyone actually using those methods before would now still be getting a multiple actions error since we now have both actions available again. Only reason why we are not seeing it in the backoffice now is because we're calling the new
see this SO post on webapi and params - there's really no good way to do what you're proposing: http://stackoverflow.com/questions/14353466/overload-web-api-action-method-based-on-parameter-type
Good point, we might (as Shannon said) be able to use a custom binder for that parameter instead.
I've 'fixed' this so that we can have 3 methods:
MediaTypeController.GetAllowedChildren(int contentId) MediaTypeController.GetAllowedChildren(Guid contentId) \MediaTypeController.GetAllowedChildren(string contentId)
MediaController.GetChildren(int id) MediaController.GetChildren(Guid id) MediaController.GetChildren(string id)
This is possible by using a custom ApiControllerActionSelector. This selector will check what type is being passed in from the client and will choose the correct method accordingly. This means that there is no breaking changes from 7.5.7 since the same signature exists and we don't have to change any of the client code. This also means that if someone updated their signature to string in 7.5.8 we won't break it in 7.5.9 (though the string ones are marked obsolete)
This can be tested with PostMan by passing in integer's or Guids for the ids.
Nice fix with the ActionSelector :) Tested and it seems to work - merged in.
Backwards Compatible: True
Affected versions: 7.5.8
Due in version: 7.5.9
Sprint: Sprint 51