Skip to content

Commit a32d38f

Browse files
ShazwazzaShazwazza
committed
Fixes: U4-5992 Caching of XPathNodeIterator in umbraco.library is not thread safe and will cause media errors like valueDictionary exception
1 parent 864994a commit a32d38f

File tree

2 files changed

+63
-48
lines changed

2 files changed

+63
-48
lines changed

‎src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedMediaCache.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,10 @@ private IPublishedContent GetUmbracoMedia(int id)
191191
}
192192
}
193193

194+
LogHelper.Debug<PublishedMediaCache>(
195+
"Could not retrieve media {0} from Examine index, reverting to looking up media via legacy library.GetMedia method",
196+
() => id);
197+
194198
var media = global::umbraco.library.GetMedia(id, true);
195199
if (media != null && media.Current != null)
196200
{
@@ -212,6 +216,10 @@ private IPublishedContent GetUmbracoMedia(int id)
212216
return ConvertFromXPathNavigator(media.Current);
213217
}
214218

219+
LogHelper.Debug<PublishedMediaCache>(
220+
"Could not retrieve media {0} from Examine index or from legacy library.GetMedia method",
221+
() => id);
222+
215223
return null;
216224
}
217225

‎src/Umbraco.Web/umbraco.presentation/library.cs

Lines changed: 55 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,39 @@
11
using System;
22
using System.Globalization;
33
using System.IO;
4+
using System.Linq;
45
using System.Net;
56
using System.Net.Mail;
67
using System.Text;
78
using System.Text.RegularExpressions;
89
using System.Web;
910
using System.Web.UI;
1011
using System.Xml;
12+
using System.Xml.Linq;
1113
using System.Xml.XPath;
14+
using Newtonsoft.Json;
1215
using Umbraco.Core;
1316
using Umbraco.Core.Cache;
17+
using Umbraco.Core.Configuration;
1418
using Umbraco.Core.Logging;
19+
using Umbraco.Core.Models;
20+
using Umbraco.Core.Services;
1521
using Umbraco.Web;
1622
using Umbraco.Web.Cache;
17-
using Umbraco.Web.PublishedCache;
18-
using Umbraco.Web.Routing;
1923
using Umbraco.Web.Templates;
2024
using umbraco.BusinessLogic;
2125
using umbraco.cms.businesslogic;
22-
using umbraco.cms.businesslogic.media;
23-
using umbraco.cms.businesslogic.member;
24-
using umbraco.cms.businesslogic.propertytype;
25-
using umbraco.cms.businesslogic.relation;
2626
using umbraco.cms.businesslogic.web;
2727
using umbraco.cms.helpers;
28-
using umbraco.presentation.cache;
2928
using umbraco.scripting;
3029
using umbraco.DataLayer;
31-
using System.Web.Security;
32-
using umbraco.cms.businesslogic.language;
3330
using Umbraco.Core.IO;
34-
using System.Collections;
35-
using System.Collections.Generic;
36-
using umbraco.cms.businesslogic.cache;
37-
using umbraco.NodeFactory;
31+
using Language = umbraco.cms.businesslogic.language.Language;
32+
using Media = umbraco.cms.businesslogic.media.Media;
33+
using Member = umbraco.cms.businesslogic.member.Member;
34+
using PropertyType = umbraco.cms.businesslogic.propertytype.PropertyType;
35+
using Relation = umbraco.cms.businesslogic.relation.Relation;
3836
using UmbracoContext = umbraco.presentation.UmbracoContext;
39-
using System.Linq;
4037

4138
namespace umbraco
4239
{
@@ -470,46 +467,48 @@ public static XPathNodeIterator GetMedia(int MediaId, bool Deep)
470467
{
471468
if (UmbracoSettings.UmbracoLibraryCacheDuration > 0)
472469
{
473-
XPathNodeIterator retVal = ApplicationContext.Current.ApplicationCache.GetCacheItem(
470+
var xml = ApplicationContext.Current.ApplicationCache.GetCacheItem(
474471
string.Format(
475472
"{0}_{1}_{2}", CacheKeys.MediaCacheKey, MediaId, Deep),
476473
TimeSpan.FromSeconds(UmbracoSettings.UmbracoLibraryCacheDuration),
477474
() => GetMediaDo(MediaId, Deep));
478475

479-
if (retVal != null)
480-
return retVal;
476+
if (xml != null)
477+
{
478+
//returning the root element of the Media item fixes the problem
479+
return xml.CreateNavigator().Select("/");
480+
}
481+
481482
}
482483
else
483484
{
484-
return GetMediaDo(MediaId, Deep);
485+
var xml = GetMediaDo(MediaId, Deep);
486+
487+
//returning the root element of the Media item fixes the problem
488+
return xml.CreateNavigator().Select("/");
485489
}
486-
487490
}
488-
catch
491+
catch(Exception ex)
489492
{
493+
LogHelper.Error<library>("An error occurred looking up media", ex);
490494
}
491495

492-
XmlDocument xd = new XmlDocument();
496+
LogHelper.Debug<library>("No media result for id {0}", () => MediaId);
497+
498+
var xd = new XmlDocument();
493499
xd.LoadXml(string.Format("<error>No media is maching '{0}'</error>", MediaId));
494500
return xd.CreateNavigator().Select("/");
495501
}
496502

497-
private static XPathNodeIterator GetMediaDo(int mediaId, bool deep)
503+
private static XElement GetMediaDo(int mediaId, bool deep)
498504
{
499-
var m = new Media(mediaId);
500-
if (m.nodeObjectType == Media._objectType)
501-
{
502-
var mXml = new XmlDocument();
503-
var xml = m.ToXml(mXml, deep);
504-
//This will be null if the media isn't public (meaning it is in the trash)
505-
if (xml == null) return null;
506-
//TODO: This is an aweful way of loading in XML - it is very slow.
507-
mXml.LoadXml(xml.OuterXml);
508-
var xp = mXml.CreateNavigator();
509-
var xpath = UmbracoSettings.UseLegacyXmlSchema ? "/node" : String.Format("/{0}", Casing.SafeAliasWithForcingCheck(m.ContentType.Alias));
510-
return xp.Select(xpath);
511-
}
512-
return null;
505+
var media = ApplicationContext.Current.Services.MediaService.GetById(mediaId);
506+
if (media == null) return null;
507+
var serializer = new EntityXmlSerializer();
508+
var serialized = serializer.Serialize(
509+
ApplicationContext.Current.Services.MediaService, ApplicationContext.Current.Services.DataTypeService,
510+
media, deep);
511+
return serialized;
513512
}
514513

515514
/// <summary>
@@ -525,35 +524,42 @@ public static XPathNodeIterator GetMember(int MemberId)
525524
{
526525
if (UmbracoSettings.UmbracoLibraryCacheDuration > 0)
527526
{
528-
var retVal = ApplicationContext.Current.ApplicationCache.GetCacheItem(
527+
var xml = ApplicationContext.Current.ApplicationCache.GetCacheItem(
529528
string.Format(
530529
"{0}_{1}", CacheKeys.MemberLibraryCacheKey, MemberId),
531530
TimeSpan.FromSeconds(UmbracoSettings.UmbracoLibraryCacheDuration),
532531
() => GetMemberDo(MemberId));
533532

534-
if (retVal != null)
535-
return retVal.CreateNavigator().Select("/");
533+
if (xml != null)
534+
{
535+
return xml.CreateNavigator().Select("/");
536+
}
536537
}
537538
else
538539
{
539540
return GetMemberDo(MemberId).CreateNavigator().Select("/");
540541
}
541-
542542
}
543-
catch
543+
catch (Exception ex)
544544
{
545+
LogHelper.Error<library>("An error occurred looking up member", ex);
545546
}
546-
XmlDocument xd = new XmlDocument();
547+
548+
LogHelper.Debug<library>("No member result for id {0}", () => MemberId);
549+
550+
var xd = new XmlDocument();
547551
xd.LoadXml(string.Format("<error>No member is maching '{0}'</error>", MemberId));
548552
return xd.CreateNavigator().Select("/");
549553
}
550554

551-
private static XmlDocument GetMemberDo(int MemberId)
555+
private static XElement GetMemberDo(int MemberId)
552556
{
553-
Member m = new Member(MemberId);
554-
XmlDocument mXml = new XmlDocument();
555-
mXml.LoadXml(m.ToXml(mXml, false).OuterXml);
556-
return mXml;
557+
var member = ApplicationContext.Current.Services.MemberService.GetById(MemberId);
558+
if (member == null) return null;
559+
var serializer = new EntityXmlSerializer();
560+
var serialized = serializer.Serialize(
561+
ApplicationContext.Current.Services.DataTypeService, member);
562+
return serialized;
557563
}
558564

559565
/// <summary>
@@ -1352,8 +1358,9 @@ public static XPathNodeIterator GetXmlNodeCurrent()
13521358
nav.MoveToId(HttpContext.Current.Items["pageID"].ToString());
13531359
return nav.Select(".");
13541360
}
1355-
catch
1361+
catch (Exception ex)
13561362
{
1363+
LogHelper.Error<library>("Could not retrieve current xml node", ex);
13571364
}
13581365

13591366
XmlDocument xd = new XmlDocument();

0 commit comments

Comments
 (0)