yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bugfixes for MNTP / EntityController (#11685)

Browse Source 
* Added EntityController.GetUrlsByIds support for int & guid + update MNTP

Fixes issue with MNTP (for 8.18) in a partial view macro - GH #11631

Renamed GetUrlsByUdis to match, don't do this in v9 as it would be
breaking there, instead mark it obsolete.

TODO: v9 ensure integration test coverage, more painful here as no
WebApplicationFactory.

* Added partial test coverage for GetUrlsByIds.

This doesn't actually work in the backoffice because of GH #11448

So lets fix that next.

* Failing test demonstrating #11448

* Fix for #11448 getByIds doesn't work as expected.

ParameterSwapControllerActionSelectorAttribute - cached body survived between requests.

* Expand on sync vs async comment for future confused souls.

* Might aswell cache parsed json vs string for performance

* Make ParameterSwapControllerActionSelector remarks more accurate.

* Share deserialized request body between action constraint and model binder

* Be more defensive with RequestBodyAsJObject HttpContext item

Only store if deserialize success.
Don't assume key being present means can cast as JObject.

* Nest constant a little deeper.

* Final defensive tweak
This commit is contained in:
Paul Johnson
2021-11-25 12:47:59 +00:00
committed by GitHub
parent b58a0cf0a5
commit 6a2a5d7fc9
8 changed files with 772 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

174
src/Umbraco.Web.BackOffice/Controllers/EntityController.cs
View File

@@ -54,6 +54,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
[ParameterSwapControllerActionSelector(nameof(GetById), "id", typeof(int), typeof(Guid), typeof(Udi))]
[ParameterSwapControllerActionSelector(nameof(GetByIds), "ids", typeof(int[]), typeof(Guid[]), typeof(Udi[]))]
[ParameterSwapControllerActionSelector(nameof(GetUrl), "id", typeof(int), typeof(Udi))]
[ParameterSwapControllerActionSelector(nameof(GetUrlsByIds), "ids", typeof(int[]), typeof(Guid[]), typeof(Udi[]))]
public class EntityController : UmbracoAuthorizedJsonController
{
private static readonly string[] _postFilterSplitStrings = { "=", "==", "!=", "<>", ">", "<", ">=", "<=" };
@@ -318,6 +319,145 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
return GetUrl(intId.Result, entityType, culture);
}
/// <summary>
/// Get entity URLs by IDs
/// </summary>
/// <param name="ids">
/// A list of IDs to lookup items by
/// </param>
/// <param name="type">The entity type to look for.</param>
/// <param name="culture">The culture to fetch the URL for.</param>
/// <returns>Dictionary mapping Udi -> Url</returns>
/// <remarks>
/// We allow for POST because there could be quite a lot of Ids.
/// </remarks>
[HttpGet]
[HttpPost]
public IDictionary<int, string> GetUrlsByIds([FromJsonPath] int[] ids, [FromQuery] UmbracoEntityTypes type, [FromQuery] string culture = null)
{
if (ids == null || !ids.Any())
{
return new Dictionary<int, string>();
}
string MediaOrDocumentUrl(int id)
{
switch (type)
{
case UmbracoEntityTypes.Document:
return _publishedUrlProvider.GetUrl(id, culture: culture ?? ClientCulture());
case UmbracoEntityTypes.Media:
{
IPublishedContent media = _publishedContentQuery.Media(id);
// NOTE: If culture is passed here we get an empty string rather than a media item URL.
return _publishedUrlProvider.GetMediaUrl(media, culture: null);
}
default:
return null;
}
}
return ids
.Distinct()
.Select(id => new {
Id = id,
Url = MediaOrDocumentUrl(id)
}).ToDictionary(x => x.Id, x => x.Url);
}
/// <summary>
/// Get entity URLs by IDs
/// </summary>
/// <param name="ids">
/// A list of IDs to lookup items by
/// </param>
/// <param name="type">The entity type to look for.</param>
/// <param name="culture">The culture to fetch the URL for.</param>
/// <returns>Dictionary mapping Udi -> Url</returns>
/// <remarks>
/// We allow for POST because there could be quite a lot of Ids.
/// </remarks>
[HttpGet]
[HttpPost]
public IDictionary<Guid, string> GetUrlsByIds([FromJsonPath] Guid[] ids, [FromQuery] UmbracoEntityTypes type, [FromQuery] string culture = null)
{
if (ids == null || !ids.Any())
{
return new Dictionary<Guid, string>();
}
string MediaOrDocumentUrl(Guid id)
{
return type switch
{
UmbracoEntityTypes.Document => _publishedUrlProvider.GetUrl(id, culture: culture ?? ClientCulture()),
// NOTE: If culture is passed here we get an empty string rather than a media item URL.
UmbracoEntityTypes.Media => _publishedUrlProvider.GetMediaUrl(id, culture: null),
_ => null
};
}
return ids
.Distinct()
.Select(id => new {
Id = id,
Url = MediaOrDocumentUrl(id)
}).ToDictionary(x => x.Id, x => x.Url);
}
/// <summary>
/// Get entity URLs by IDs
/// </summary>
/// <param name="ids">
/// A list of IDs to lookup items by
/// </param>
/// <param name="type">The entity type to look for.</param>
/// <param name="culture">The culture to fetch the URL for.</param>
/// <returns>Dictionary mapping Udi -> Url</returns>
/// <remarks>
/// We allow for POST because there could be quite a lot of Ids.
/// </remarks>
[HttpGet]
[HttpPost]
public IDictionary<Udi, string> GetUrlsByIds([FromJsonPath] Udi[] ids, [FromQuery] UmbracoEntityTypes type, [FromQuery] string culture = null)
{
if (ids == null || !ids.Any())
{
return new Dictionary<Udi, string>();
}
// TODO: PMJ 2021-09-27 - Should GetUrl(Udi) exist as an extension method on UrlProvider/IUrlProvider (in v9)
string MediaOrDocumentUrl(Udi id)
{
if (id is not GuidUdi guidUdi)
{
return null;
}
return type switch
{
UmbracoEntityTypes.Document => _publishedUrlProvider.GetUrl(guidUdi.Guid, culture: culture ?? ClientCulture()),
// NOTE: If culture is passed here we get an empty string rather than a media item URL.
UmbracoEntityTypes.Media => _publishedUrlProvider.GetMediaUrl(guidUdi.Guid, culture: null),
_ => null
};
}
return ids
.Distinct()
.Select(id => new {
Id = id,
Url = MediaOrDocumentUrl(id)
}).ToDictionary(x => x.Id, x => x.Url);
}
/// <summary>
/// Get entity URLs by UDIs
/// </summary>
@@ -331,33 +471,31 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
/// </remarks>
[HttpGet]
[HttpPost]
[Obsolete("Use GetUrlsByIds instead.")]
public IDictionary<Udi, string> GetUrlsByUdis([FromJsonPath] Udi[] udis, string culture = null)
{
if (udis == null || udis.Length == 0)
if (udis == null || !udis.Any())
{
return new Dictionary<Udi, string>();
}
// TODO: PMJ 2021-09-27 - Should GetUrl(Udi) exist as an extension method on UrlProvider/IUrlProvider (in v9)
string MediaOrDocumentUrl(Udi udi)
{
if (udi is not GuidUdi guidUdi)
{
return null;
}
var udiEntityType = udis.First().EntityType;
UmbracoEntityTypes entityType;
return guidUdi.EntityType switch
{
Constants.UdiEntityType.Document => _publishedUrlProvider.GetUrl(guidUdi.Guid,
culture: culture ?? ClientCulture()),
// NOTE: If culture is passed here we get an empty string rather than a media item URL WAT
Constants.UdiEntityType.Media => _publishedUrlProvider.GetMediaUrl(guidUdi.Guid, culture: null),
_ => null
};
switch (udiEntityType)
{
case Constants.UdiEntityType.Document:
entityType = UmbracoEntityTypes.Document;
break;
case Constants.UdiEntityType.Media:
entityType = UmbracoEntityTypes.Media;
break;
default:
entityType = (UmbracoEntityTypes)(-1);
break;
}
return udis
.Select(udi => new { Udi = udi, Url = MediaOrDocumentUrl(udi) }).ToDictionary(x => x.Udi, x => x.Url);
return GetUrlsByIds(udis, entityType, culture);
}
/// <summary>

80
src/Umbraco.Web.BackOffice/Controllers/ParameterSwapControllerActionSelectorAttribute.cs
View File

@@ -1,22 +1,48 @@
using System;
using System;
using System.Linq;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.ActionConstraints;
using Microsoft.AspNetCore.Mvc.Controllers;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Umbraco.Cms.Core;
using Umbraco.Extensions;
namespace Umbraco.Cms.Web.BackOffice.Controllers
{
/// <remarks>
/// <para>
/// This attribute is odd because it applies at class level where some methods may use it whilst others don't.
/// </para>
///
/// <para>
/// What we should probably have (if we really even need something like this at all) is an attribute for method level.
///
/// <example>
/// <code>
///
/// [HasParameterFromUriOrBodyOfType("ids", typeof(Guid[]))]
/// public IActionResult GetByIds([FromJsonPath] Guid[] ids) { }
///
/// [HasParameterFromUriOrBodyOfType("ids", typeof(int[]))]
/// public IActionResult GetByIds([FromJsonPath] int[] ids) { }
/// </code>
/// </example>
/// </para>
///
/// <para>
/// That way we wouldn't need confusing things like Accept returning true when action name doesn't even match attribute metadata.
/// </para>
/// </remarks>
[AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = true)]
internal class ParameterSwapControllerActionSelectorAttribute : Attribute, IActionConstraint
{
private readonly string _actionName;
private readonly string _parameterName;
private readonly Type[] _supportedTypes;
private string _requestBody;
public ParameterSwapControllerActionSelectorAttribute(string actionName, string parameterName, params Type[] supportedTypes)
{
@@ -33,10 +59,12 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
{
if (!IsValidCandidate(context.CurrentCandidate))
{
// See remarks on class, required because we apply at class level
// and some controllers have some actions with parameter swaps and others without.
return true;
}
var chosenCandidate = SelectAction(context);
ActionSelectorCandidate? chosenCandidate = SelectAction(context);
var found = context.CurrentCandidate.Equals(chosenCandidate);
return found;
@@ -49,23 +77,45 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
return candidate;
}
HttpContext httpContext = context.RouteContext.HttpContext;
// if it's a post we can try to read from the body and bind from the json value
if (context.RouteContext.HttpContext.Request.Method == HttpMethod.Post.ToString())
if (context.RouteContext.HttpContext.Request.Method.Equals(HttpMethod.Post.Method))
{
// We need to use the asynchronous method here if synchronous IO is not allowed (it may or may not be, depending
// on configuration in UmbracoBackOfficeServiceCollectionExtensions.AddUmbraco()).
// We can't use async/await due to the need to override IsValidForRequest, which doesn't have an async override, so going with
// this, which seems to be the least worst option for "sync to async" (https://stackoverflow.com/a/32429753/489433).
var strJson = _requestBody ??= Task.Run(() => context.RouteContext.HttpContext.Request.GetRawBodyStringAsync()).GetAwaiter().GetResult();
JObject postBodyJson;
var json = JsonConvert.DeserializeObject<JObject>(strJson);
if (httpContext.Items.TryGetValue(Constants.HttpContext.Items.RequestBodyAsJObject, out var value) && value is JObject cached)
{
postBodyJson = cached;
}
else
{
// We need to use the asynchronous method here if synchronous IO is not allowed (it may or may not be, depending
// on configuration in UmbracoBackOfficeServiceCollectionExtensions.AddUmbraco()).
// We can't use async/await due to the need to override IsValidForRequest, which doesn't have an async override, so going with
// this, which seems to be the least worst option for "sync to async" (https://stackoverflow.com/a/32429753/489433).
//
// To expand on the above, if KestrelServerOptions/IISServerOptions is AllowSynchronousIO=false
// And you attempt to read stream sync an InvalidOperationException is thrown with message
// "Synchronous operations are disallowed. Call ReadAsync or set AllowSynchronousIO to true instead."
var rawBody = Task.Run(() => httpContext.Request.GetRawBodyStringAsync()).GetAwaiter().GetResult();
try
{
postBodyJson = JsonConvert.DeserializeObject<JObject>(rawBody);
httpContext.Items[Constants.HttpContext.Items.RequestBodyAsJObject] = postBodyJson;
}
catch (JsonException)
{
postBodyJson = null;
}
}
if (json == null)
if (postBodyJson == null)
{
return null;
}
var requestParam = json[_parameterName];
var requestParam = postBodyJson[_parameterName];
if (requestParam != null)
{
@@ -85,11 +135,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
}
}
}
catch (JsonReaderException)
{
// can't convert
}
catch (JsonSerializationException)
catch (JsonException)
{
// can't convert
}