Fixes and documents the odd async usage for reading multipart in a model binder, fixes up navigation.service checks and adds unit tests for that

This commit is contained in:
Shannon
2018-07-17 15:33:57 +10:00
parent e9752cd5e1
commit e2bcf59b0f
3 changed files with 95 additions and 13 deletions

View File

@@ -153,9 +153,11 @@ function navigationService($rootScope, $route, $routeParams, $log, $location, $q
//If any of the other parts have changed we do not cancel
var currRoutingKeys = _.difference(_.keys(currUrlParams), nonRoutingQueryStrings);
var nextRoutingKeys = _.difference(_.keys(nextUrlParams), nonRoutingQueryStrings);
var diff = _.difference(currRoutingKeys, nextRoutingKeys);
var diff1 = _.difference(currRoutingKeys, nextRoutingKeys);
var diff2 = _.difference(nextRoutingKeys, currRoutingKeys);
//if the routing parameter keys are the same, we'll compare their values to see if any have changed and if so then the routing will be allowed.
if (diff.length == 0) {
if (diff1.length === 0 && diff2.length === 0) {
var partsChanged = 0;
_.each(currRoutingKeys, function (k) {
if (currUrlParams[k] != nextUrlParams[k]) {

View File

@@ -0,0 +1,50 @@
describe('navigation services tests', function () {
var navigationService;
beforeEach(module('umbraco.services'));
beforeEach(module('ngRoute'));
beforeEach(inject(function ($injector) {
navigationService = $injector.get('navigationService');
}));
describe('determine if route change causes navigation', function () {
it('navigates when parameters added', function () {
var currParams = { section: "content", id: 123 };
var nextParams = { section: "content", id: 123, create: true };
var result = navigationService.isRouteChangingNavigation(currParams, nextParams);
expect(result).toBe(true);
});
it('navigates when parameters removed', function () {
var currParams = { section: "content", id: 123, create: true };
var nextParams = { section: "content", id: 123 };
var result = navigationService.isRouteChangingNavigation(currParams, nextParams);
expect(result).toBe(true);
});
it('does not navigate when non routing parameters added', function () {
var currParams = { section: "content", id: 123 };
var nextParams = { section: "content", id: 123, mculture: "abc", cculture: "xyz" };
var result = navigationService.isRouteChangingNavigation(currParams, nextParams);
expect(result).toBe(false);
});
it('does not navigate when non routing parameters changed', function () {
var currParams = { section: "content", id: 123, mculture: "abc" };
var nextParams = { section: "content", id: 123, mculture: "ooo", cculture: "xyz" };
var result = navigationService.isRouteChangingNavigation(currParams, nextParams);
expect(result).toBe(false);
});
it('does not navigate when non routing parameters removed', function () {
var currParams = { section: "content", id: 123, mculture: "abc", cculture: "xyz" };
var nextParams = { section: "content", id: 123, cculture: "xyz" };
var result = navigationService.isRouteChangingNavigation(currParams, nextParams);
expect(result).toBe(false);
});
});
});

View File

@@ -66,12 +66,9 @@ namespace Umbraco.Web.WebApi.Binders
throw new HttpResponseException(HttpStatusCode.UnsupportedMediaType);
}
var root = IOHelper.MapPath("~/App_Data/TEMP/FileUploads");
//ensure it exists
Directory.CreateDirectory(root);
var provider = new MultipartFormDataStreamProvider(root);
var result = GetMultiPartResult(actionContext);
var model = GetModel(actionContext, bindingContext, provider);
var model = GetModel(actionContext, bindingContext, result);
//now that everything is binded, validate the properties
var contentItemValidator = GetValidationHelper();
contentItemValidator.ValidateItem(actionContext, model);
@@ -85,6 +82,41 @@ namespace Umbraco.Web.WebApi.Binders
return new ContentItemValidationHelper<TPersisted, TModelSave>(Logger, UmbracoContextAccessor);
}
private MultipartFormDataStreamProvider GetMultiPartResult(HttpActionContext actionContext)
{
var root = IOHelper.MapPath("~/App_Data/TEMP/FileUploads");
//ensure it exists
Directory.CreateDirectory(root);
var provider = new MultipartFormDataStreamProvider(root);
var request = actionContext.Request;
var content = request.Content;
//NOTE: Yes this is super strange, ugly and weird. One would think that you could 'just'
// use a .Return on the ReadAsMultipartAsync but that absolutely doesn't work, it immediately deadlocks.
// We've tried all things like .Result, ConfigureAwait(false), GetAwaiter().GetResult(), a combination of
// everything and they all immediately deadlock.
// This issue has been documented in various .NET releases but there's really no info about the best way
// to do this. Ideally we'd use true async/await but in .NET Framework, model binders don't support async :(
// For some reason wrapping the operation in our own task works without deadlocking.
MultipartFormDataStreamProvider result = null;
var task = Task.Run(() => content.ReadAsMultipartAsync(provider))
.ContinueWith(x =>
{
if (x.IsFaulted && x.Exception != null)
{
throw x.Exception;
}
result = x.ConfigureAwait(false).GetAwaiter().GetResult();
});
task.Wait();
if (result == null)
throw new InvalidOperationException("Could not read multi-part message");
return result;
}
/// <summary>
/// Builds the model from the request contents
/// </summary>
@@ -92,13 +124,11 @@ namespace Umbraco.Web.WebApi.Binders
/// <param name="bindingContext"></param>
/// <param name="provider"></param>
/// <returns></returns>
private TModelSave GetModel(HttpActionContext actionContext, ModelBindingContext bindingContext, MultipartFormDataStreamProvider provider)
private TModelSave GetModel(HttpActionContext actionContext, ModelBindingContext bindingContext, MultipartFormDataStreamProvider result)
{
var request = actionContext.Request;
var content = request.Content;
var result = content.ReadAsMultipartAsync(provider).Result;
//var request = actionContext.Request;
//var content = request.Content;
//var result = content.ReadAsMultipartAsync(provider).ConfigureAwait(false).GetAwaiter().GetResult();
if (result.FormData["contentItem"] == null)
{