diff --git a/src/Umbraco.Core/Services/IPublicAccessService.cs b/src/Umbraco.Core/Services/IPublicAccessService.cs index 617a8f220d..7f7da72c62 100644 --- a/src/Umbraco.Core/Services/IPublicAccessService.cs +++ b/src/Umbraco.Core/Services/IPublicAccessService.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using Umbraco.Core.Models; using Umbraco.Core.Security; @@ -42,14 +43,18 @@ namespace Umbraco.Core.Services /// Attempt IsProtected(string contentPath); + [EditorBrowsable(EditorBrowsableState.Never)] + [Obsolete("Use AddRule instead, this method will be removed in future versions")] + Attempt> AddOrUpdateRule(IContent content, string ruleType, string ruleValue); + /// - /// Adds/updates a rule, if an entry doesn't exist one will be created with the new rule + /// Adds a rule if the entry doesn't already exist /// /// /// - /// + /// /// - Attempt> AddOrUpdateRule(IContent content, string ruleType, string newRuleValue); + Attempt> AddRule(IContent content, string ruleType, string ruleValue); /// /// Removes a rule diff --git a/src/Umbraco.Core/Services/PublicAccessService.cs b/src/Umbraco.Core/Services/PublicAccessService.cs index 2f033740fa..bb966309dc 100644 --- a/src/Umbraco.Core/Services/PublicAccessService.cs +++ b/src/Umbraco.Core/Services/PublicAccessService.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Linq; using Umbraco.Core.Events; using Umbraco.Core.Logging; @@ -104,14 +105,21 @@ namespace Umbraco.Core.Services return Attempt.If(result != null, result); } + [EditorBrowsable(EditorBrowsableState.Never)] + [Obsolete("Use AddRule instead, this method will be removed in future versions")] + public Attempt> AddOrUpdateRule(IContent content, string ruleType, string ruleValue) + { + return AddRule(content, ruleType, ruleValue); + } + /// - /// Adds/updates a rule + /// Adds a rule /// /// /// - /// + /// /// - public Attempt> AddOrUpdateRule(IContent content, string ruleType, string newRuleValue) + public Attempt> AddRule(IContent content, string ruleType, string ruleValue) { var evtMsgs = EventMessagesFactory.Get(); var uow = UowProvider.GetUnitOfWork(); @@ -121,15 +129,16 @@ namespace Umbraco.Core.Services if (entry == null) return Attempt>.Fail(); - var existingRule = entry.Rules.FirstOrDefault(x => x.RuleType == ruleType && x.RuleValue == newRuleValue); + var existingRule = entry.Rules.FirstOrDefault(x => x.RuleType == ruleType && x.RuleValue == ruleValue); if (existingRule == null) { - entry.AddRule(newRuleValue, ruleType); + entry.AddRule(ruleValue, ruleType); } else { - existingRule.RuleType = ruleType; - existingRule.RuleValue = newRuleValue; + //If they are both the same already then there's nothing to update, exit + return Attempt>.Succeed( + new OperationStatus(entry, OperationStatusType.Success, evtMsgs)); } if (Saving.IsRaisedEventCancelled( diff --git a/src/Umbraco.Tests/Services/PublicAccessServiceTests.cs b/src/Umbraco.Tests/Services/PublicAccessServiceTests.cs index 8f5a37376c..57fe377262 100644 --- a/src/Umbraco.Tests/Services/PublicAccessServiceTests.cs +++ b/src/Umbraco.Tests/Services/PublicAccessServiceTests.cs @@ -80,7 +80,7 @@ namespace Umbraco.Tests.Services publicAccessService.Save(entry); // Act - var updated = publicAccessService.AddOrUpdateRule(c, "TestType2", "AnotherVal"); + var updated = publicAccessService.AddRule(c, "TestType2", "AnotherVal"); //re-get entry = publicAccessService.GetEntryForContent(c); @@ -90,6 +90,42 @@ namespace Umbraco.Tests.Services Assert.AreEqual(2, entry.Rules.Count()); } + [Test] + public void Can_Add_Multiple_Value_For_Same_Rule_Type() + { + // Arrange + var contentService = ServiceContext.ContentService; + var contentTypeService = ServiceContext.ContentTypeService; + var ct = MockedContentTypes.CreateSimpleContentType("blah", "Blah"); + contentTypeService.Save(ct); + var c = MockedContent.CreateSimpleContent(ct, "Test", -1); + contentService.Save(c); + var publicAccessService = ServiceContext.PublicAccessService; + var entry = new PublicAccessEntry(c, c, c, new[] + { + new PublicAccessRule() + { + RuleType = "TestType", + RuleValue = "TestVal" + }, + }); + publicAccessService.Save(entry); + + // Act + var updated1 = publicAccessService.AddRule(c, "TestType", "AnotherVal1"); + var updated2 = publicAccessService.AddRule(c, "TestType", "AnotherVal2"); + + //re-get + entry = publicAccessService.GetEntryForContent(c); + + // Assert + Assert.IsTrue(updated1.Success); + Assert.IsTrue(updated2.Success); + Assert.AreEqual(OperationStatusType.Success, updated1.Result.StatusType); + Assert.AreEqual(OperationStatusType.Success, updated2.Result.StatusType); + Assert.AreEqual(3, entry.Rules.Count()); + } + [Test] public void Can_Remove_Rule() { @@ -106,14 +142,18 @@ namespace Umbraco.Tests.Services new PublicAccessRule() { RuleType = "TestType", - RuleValue = "TestValue" + RuleValue = "TestValue1" + }, + new PublicAccessRule() + { + RuleType = "TestType", + RuleValue = "TestValue2" }, }); - publicAccessService.Save(entry); - publicAccessService.AddOrUpdateRule(c, "TestType", "AnotherVal"); + publicAccessService.Save(entry); // Act - var removed = publicAccessService.RemoveRule(c, "TestType", "TestValue"); + var removed = publicAccessService.RemoveRule(c, "TestType", "TestValue1"); //re-get entry = publicAccessService.GetEntryForContent(c); @@ -121,7 +161,8 @@ namespace Umbraco.Tests.Services Assert.IsTrue(removed.Success); Assert.AreEqual(OperationStatusType.Success, removed.Result.StatusType); Assert.AreEqual(1, entry.Rules.Count()); - Assert.AreEqual("AnotherVal", entry.Rules.ElementAt(0).RuleValue); + Assert.AreEqual("TestValue2", entry.Rules.ElementAt(0).RuleValue); } + } } \ No newline at end of file diff --git a/src/umbraco.cms/businesslogic/web/Access.cs b/src/umbraco.cms/businesslogic/web/Access.cs index b2d4b4c103..bcbbbb9c69 100644 --- a/src/umbraco.cms/businesslogic/web/Access.cs +++ b/src/umbraco.cms/businesslogic/web/Access.cs @@ -71,7 +71,7 @@ namespace umbraco.cms.businesslogic.web if (e.Cancel) return; - var entry = ApplicationContext.Current.Services.PublicAccessService.AddOrUpdateRule( + var entry = ApplicationContext.Current.Services.PublicAccessService.AddRule( doc.ContentEntity, Constants.Conventions.PublicAccess.MemberRoleRuleType, role); @@ -95,7 +95,7 @@ namespace umbraco.cms.businesslogic.web if (content == null) throw new Exception("No content found with document id " + DocumentId); - if (ApplicationContext.Current.Services.PublicAccessService.AddOrUpdateRule( + if (ApplicationContext.Current.Services.PublicAccessService.AddRule( content, Constants.Conventions.PublicAccess.MemberGroupIdRuleType, MemberGroupId.ToString(CultureInfo.InvariantCulture))) @@ -113,7 +113,7 @@ namespace umbraco.cms.businesslogic.web if (content == null) throw new Exception("No content found with document id " + DocumentId); - if (ApplicationContext.Current.Services.PublicAccessService.AddOrUpdateRule( + if (ApplicationContext.Current.Services.PublicAccessService.AddRule( content, Constants.Conventions.PublicAccess.MemberIdRuleType, MemberId.ToString(CultureInfo.InvariantCulture))) @@ -132,7 +132,7 @@ namespace umbraco.cms.businesslogic.web if (e.Cancel) return; - var entry = ApplicationContext.Current.Services.PublicAccessService.AddOrUpdateRule( + var entry = ApplicationContext.Current.Services.PublicAccessService.AddRule( doc.ContentEntity, Constants.Conventions.PublicAccess.MemberUsernameRuleType, membershipUserName); @@ -155,7 +155,7 @@ namespace umbraco.cms.businesslogic.web { var doc = new Document(DocumentId); - var entry = ApplicationContext.Current.Services.PublicAccessService.AddOrUpdateRule( + var entry = ApplicationContext.Current.Services.PublicAccessService.AddRule( doc.ContentEntity, Constants.Conventions.PublicAccess.MemberGroupIdRuleType, MemberGroupId.ToString(CultureInfo.InvariantCulture));