Make readonly collections truly readonly

These methods actually returned mutable collections that happen
to implement the readonly interfaces. User could cast them to
the actual type and add things. I'd rather avoid even that
possibility by making these truly return readonly stuff.
This commit is contained in:
Haacked
2014-02-26 18:38:57 -08:00
parent 116dd47ab4
commit 9eb14f3c51
8 changed files with 94 additions and 13 deletions

View File

@@ -1,30 +1,31 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using Octokit.Response;
using Xunit;
namespace Octokit.Tests.Models
public class PunchCardTests
{
public class PunchCardTests
public class TheConstructor
{
[Fact]
public void ThrowsExceptionWithNullPunchCardPoints()
{
Assert.Throws<ArgumentNullException>(()=>new PunchCard(null));
Assert.Throws<ArgumentNullException>(() => new PunchCard(null));
}
[Fact]
public void ThrowsExceptionWhenPunchCardPointsHaveIncorrectFormat()
{
IList<int> point1 = new []{1,2,3,4,5,6};
IEnumerable<IList<int>> points = new List<IList<int>>{point1};
IList<int> point1 = new[] { 1, 2, 3, 4, 5, 6 };
IEnumerable<IList<int>> points = new List<IList<int>> { point1 };
Assert.Throws<ArgumentException>(() => new PunchCard(points));
}
[Fact]
public void DoesNotThrowExceptionWhenPunchPointsHaveCorrectFormat()
{
IList<int> point1 = new[] { 1, 2, 3};
IList<int> point1 = new[] { 1, 2, 3 };
IEnumerable<IList<int>> points = new List<IList<int>> { point1 };
Assert.DoesNotThrow(() => new PunchCard(points));
}
@@ -35,7 +36,7 @@ namespace Octokit.Tests.Models
IList<int> point1 = new[] { 1, 0, 3 };
IList<int> point2 = new[] { 1, 1, 4 };
IList<int> point3 = new[] { 1, 2, 0 };
IEnumerable<IList<int>> points = new List<IList<int>> { point1,point2,point3 };
IEnumerable<IList<int>> points = new List<IList<int>> { point1, point2, point3 };
var punchCard = new PunchCard(points);
@@ -44,10 +45,23 @@ namespace Octokit.Tests.Models
var commitsAtMondayAt2Am = punchCard.GetCommitCountFor(DayOfWeek.Monday, 2);
var commitsAtTuesdayAt2Am = punchCard.GetCommitCountFor(DayOfWeek.Tuesday, 2);
Assert.Equal(3,commitsAtMondayAt12Am);
Assert.Equal(3, commitsAtMondayAt12Am);
Assert.Equal(4, commitsAtMondayAt1Am);
Assert.Equal(0, commitsAtMondayAt2Am);
Assert.Equal(0, commitsAtTuesdayAt2Am);
}
[Fact]
public void SetsPunchPointsAsReadOnlyDictionary()
{
IList<int> point1 = new[] { 1, 0, 3 };
IList<int> point2 = new[] { 1, 1, 4 };
IList<int> point3 = new[] { 1, 2, 0 };
IEnumerable<IList<int>> points = new List<IList<int>> { point1, point2, point3 };
var punchCard = new PunchCard(points);
Assert.Null(punchCard.PunchPoints as ICollection<PunchCardPoint>);
}
}
}
}

View File

@@ -0,0 +1,20 @@
using System.Collections.Generic;
using Octokit;
using Xunit;
public class SearchCodeRequestTests
{
public class TheMergedQualifiersMethod
{
[Fact]
public void ReturnsAReadOnlyDictionary()
{
var request = new SearchCodeRequest("test");
var result = request.MergedQualifiers();
// If I can cast this to a writeable collection, then that defeats the purpose of a read only.
Assert.Null(result as ICollection<string>);
}
}
}

View File

@@ -0,0 +1,20 @@
using System.Collections.Generic;
using Octokit;
using Xunit;
internal class SearchIssuesRequestTests
{
public class TheMergedQualifiersMethod
{
[Fact]
public void ReturnsAReadOnlyDictionary()
{
var request = new SearchIssuesRequest("test");
var result = request.MergedQualifiers();
// If I can cast this to a writeable collection, then that defeats the purpose of a read only.
Assert.Null(result as ICollection<string>);
}
}
}

View File

@@ -0,0 +1,20 @@
using System.Collections.Generic;
using Octokit;
using Xunit;
internal class SearchUsersRequestTests
{
public class TheMergedQualifiersMethod
{
[Fact]
public void ReturnsAReadOnlyDictionary()
{
var request = new SearchUsersRequest("test");
var result = request.MergedQualifiers();
// If I can cast this to a writeable collection, then that defeats the purpose of a read only.
Assert.Null(result as ICollection<string>);
}
}
}

View File

@@ -134,6 +134,9 @@
<Compile Include="Models\ReadOnlyPagedCollectionTests.cs" />
<Compile Include="Models\RepositoryUpdateTests.cs" />
<Compile Include="Models\RequestParametersTests.cs" />
<Compile Include="Models\SearchCodeRequestTests.cs" />
<Compile Include="Models\SearchUsersRequestTests.cs" />
<Compile Include="Models\SearchIssuesRequestTests.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
<Compile Include="Helpers\StringExtensionsTests.cs" />
<Compile Include="Clients\RepositoriesClientTests.cs" />

View File

@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
@@ -157,7 +158,7 @@ namespace Octokit
parameters.Add(String.Format(CultureInfo.InvariantCulture, "repo:{0}", Repo));
}
return parameters;
return new ReadOnlyCollection<string>(parameters);
}
internal string DebuggerDisplay

View File

@@ -1,4 +1,5 @@
using System.Diagnostics;
using System.Collections.ObjectModel;
using System.Diagnostics;
using Octokit.Internal;
using System;
using System.Collections.Generic;
@@ -259,7 +260,7 @@ namespace Octokit
parameters.Add(String.Format(CultureInfo.InvariantCulture, "repo:{0}", Repo));
}
return parameters;
return new ReadOnlyCollection<string>(parameters);
}
internal string DebuggerDisplay

View File

@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
@@ -12,7 +13,8 @@ namespace Octokit.Response
public PunchCard(IEnumerable<IList<int>> punchCardData)
{
Ensure.ArgumentNotNull(punchCardData, "punchCardData");
PunchPoints = punchCardData.Select(point => new PunchCardPoint(point)).ToList();
PunchPoints = new ReadOnlyCollection<PunchCardPoint>(
punchCardData.Select(point => new PunchCardPoint(point)).ToList());
}
/// <summary>