Skip to content
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.

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

Performance enhancements #81

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 20 additions & 8 deletions src/RJP.MultiUrlPicker/Models/Link.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace RJP.MultiUrlPicker.Models
public class Link
{
private readonly JToken _linkItem;
private readonly Lazy<UmbracoHelper> _umbracoHelper;
private bool _publishedContentInitialized = false;
private string _name;
private string _url;
Expand All @@ -24,9 +25,10 @@ public class Link
private Udi _udi;
private int? _id;

public Link(JToken linkItem)
public Link(JToken linkItem, Lazy<UmbracoHelper> umbracoHelper = null)
{
_linkItem = linkItem;
_umbracoHelper = umbracoHelper ?? new Lazy<UmbracoHelper>(() => new UmbracoHelper(UmbracoContext.Current));
}

private IPublishedContent PublishedContent
Expand Down Expand Up @@ -158,7 +160,6 @@ public LinkType Type
}
}


private void InitPublishedContent()
{
if (!_publishedContentInitialized)
Expand All @@ -170,9 +171,22 @@ private void InitPublishedContent()
return;
}

if (Udi.TryParse(_linkItem.Value<string>("udi"), out _udi))
if (Udi.TryParse(_linkItem.Value<string>("udi"), out _udi) && _udi is GuidUdi guidUdi)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of this code is copied from the ToPublishedContent() method, but now uses the same UmbracoHelper.

{
_content = _udi.ToPublishedContent();
var umbracoType = Constants.UdiEntityType.ToUmbracoObjectType(_udi.EntityType);
if (umbracoType == UmbracoObjectTypes.Media)
{
var entityService = ApplicationContext.Current.Services.EntityService;
var mediaAttempt = entityService.GetIdForKey(guidUdi.Guid, umbracoType);
if (mediaAttempt.Success)
{
_content = _umbracoHelper.Value.TypedMedia(mediaAttempt.Result);
}
}
else
{
_content = _umbracoHelper.Value.TypedContent(guidUdi.Guid);
}
_id = _content?.Id;
}
else
Expand All @@ -181,15 +195,13 @@ private void InitPublishedContent()
_id = _linkItem.Value<int?>("id");
if (_id.HasValue)
{
var helper = new UmbracoHelper(UmbracoContext.Current);

if (_linkItem.Value<bool>("isMedia"))
{
_content = helper.TypedMedia(_id.Value);
_content = _umbracoHelper.Value.TypedMedia(_id.Value);
}
else
{
_content = helper.TypedContent(_id.Value);
_content = _umbracoHelper.Value.TypedContent(_id.Value);
}

SetUdi();
Expand Down
9 changes: 6 additions & 3 deletions src/RJP.MultiUrlPicker/Models/MultiUrls.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace RJP.MultiUrlPicker.Models
namespace RJP.MultiUrlPicker.Models
{
using System;
using System.Collections.Generic;
Expand All @@ -8,6 +8,7 @@
using Newtonsoft.Json.Linq;

using Umbraco.Core.Logging;
using Umbraco.Web;

[Obsolete("Use IEnumerable<Link> instead")]
public class MultiUrls : IEnumerable<Link>
Expand Down Expand Up @@ -39,12 +40,14 @@ public MultiUrls(string propertyData)

private void Initialize(JArray data)
{
var umbracoHelper = new Lazy<UmbracoHelper>(() => new UmbracoHelper(UmbracoContext.Current));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By using Lazy<UmbracoHelper> and passing the same instance, it is only created when actually needed and only once (if multiple links are added).


foreach (var item in data)
{
var newLink = new Link(item);
var newLink = new Link(item, umbracoHelper);
if (!newLink.Deleted)
{
_multiUrls.Add(new Link(item));
_multiUrls.Add(newLink);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just add the link, no need to create a yet another one 😄

}
else
{
Expand Down
24 changes: 13 additions & 11 deletions src/RJP.MultiUrlPicker/MultiUrlPickerValueConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,24 @@ public override bool IsConverter(PublishedPropertyType propertyType)

public override object ConvertDataToSource(PublishedPropertyType propertyType, object source, bool preview)
{
if (string.IsNullOrWhiteSpace(source?.ToString()))
var sourceString = source?.ToString();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Convert the source to a string once and then use it everywhere, no need to do the same over and over again 😀

if (string.IsNullOrWhiteSpace(sourceString))
{
return null;
}

if (source.ToString().Trim().StartsWith("["))
if (sourceString.DetectIsJson())
{
try
{
return JArray.Parse(source.ToString());
return JArray.Parse(sourceString);
}
catch (Exception ex)
{
LogHelper.Error<MultiUrlPickerValueConverter>("Error parsing JSON", ex);
}
}

return null;
}

Expand All @@ -65,14 +67,16 @@ public override object ConvertSourceToObject(PublishedPropertyType propertyType,
}

var urls = new MultiUrls((JArray)source);
if(isMultiple)
if (isMultiple)
{
if(maxNumberOfItems > 0)
if (maxNumberOfItems > 0)
{
return urls.Take(maxNumberOfItems);
}

return urls;
}

return urls.FirstOrDefault();
}

Expand All @@ -82,6 +86,7 @@ public Type GetPropertyValueType(PublishedPropertyType propertyType)
{
return typeof(IEnumerable<Link>);
}

return typeof(Link);
}

Expand All @@ -108,12 +113,9 @@ private bool IsMultipleDataType(int dataTypeId, out int maxNumberOfItems)
if (preValues.TryGetValue("maxNumberOfItems", out PreValue maxNumberOfItemsPreValue) &&
int.TryParse(maxNumberOfItemsPreValue.Value, out maxNumberOfItems))
{
PreValue versionPreValue;
Version version;
// for backwards compatibility, always return true if version
// is less than 2.0.0
if (preValues.TryGetValue("version", out versionPreValue) &&
Version.TryParse(versionPreValue.Value, out version)
// For backwards compatibility, always return true if version is less than 2.0.0
if (preValues.TryGetValue("version", out PreValue versionPreValue) &&
Version.TryParse(versionPreValue.Value, out Version version)
&& version >= new Version(2, 0, 0))
{
return maxNumberOfItems != 1;
Expand Down