Skip to content

Commit

Permalink
Fix project and group resolution in mocks (#676)
Browse files Browse the repository at this point in the history
* Pass the unescaped project id to all usages of ProjectExtensions.FindProject()

FindProject() compares the specified ProjectId value to the namespace and project names of mock projects.
This was failing because most usages were passing the value of ProjectId.ValueAsUriParameter() which is the URL-escaped form of the project path.
When the project path contained a "/", no matching project could be found, causing a NullReferenceException when trying to get a project client from the GitLabClient mock.

* Pass the unescaped group id to all usages of GroupExtensions.FindGroup()

FindGroup() compares the specified GroupId value to the namespace and names of mock groups.
This was failing because most usages were passing the value of GroupId.ValueAsUriParameter() which is the URL-escaped form of the project path.
When the group path contained a "/", no matching group could be found, causing a NullReferenceException when trying to get a group client from the GitLabClient mock.

* Fix typo in GitLabClientMockTest

Co-authored-by: Thomas Cortes <[email protected]>

* Include hint about exclusion from test cases in constructor of MergeRequestClient mock

---------

Co-authored-by: Thomas Cortes <[email protected]>
  • Loading branch information
ap0llo and Toa741 authored May 9, 2024
1 parent 48c7d2f commit bbabe0e
Show file tree
Hide file tree
Showing 26 changed files with 211 additions and 28 deletions.
126 changes: 126 additions & 0 deletions NGitLab.Mock.Tests/GitLabClientMockTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using NGitLab.Mock.Config;
using NGitLab.Models;
using NUnit.Framework;

namespace NGitLab.Mock.Tests;


public class GitLabClientMockTest
{
public static IEnumerable ProjectClientTestCases
{
get
{
static TestCaseData TestCase(string name, Func<IGitLabClient, ProjectId, object> getClient) => new TestCaseData(getClient).SetArgDisplayNames(name);

// Create a test case for each method in IGitLabClient that has a single parameter of type ProjectId
foreach (var method in GetMethods<ProjectId>())
{
yield return TestCase(method.Name, (client, id) => method.Invoke(client, new object[] { id }));
}
}
}

public static IEnumerable GroupClientTestCases
{
get
{
static TestCaseData TestCase(string name, Func<IGitLabClient, GroupId, object> getClient) => new TestCaseData(getClient).SetArgDisplayNames(name);

// Create a test case for each method in IGitLabClient that has a single parameter of type GroupId
foreach (var method in GetMethods<GroupId>())
{
// GetGroupMergeRequest() is not implemented in the mock MergeRequestClient and will throw a NotImplementedException
// To avoid a test failure, exclude this method from the test cases (this is the only such case)
// This can be removed if a mock for group merge requests is implemented
if (string.Equals(method.Name, nameof(IGitLabClient.GetGroupMergeRequest), StringComparison.Ordinal))
{
continue;
}

yield return TestCase(method.Name, (client, id) => method.Invoke(client, new object[] { id }));
}

}
}

[TestCaseSource(nameof(ProjectClientTestCases))]
public void Test_can_get_project_client(Func<IGitLabClient, ProjectId, object> getClient)
{
using var server = new GitLabConfig()
.WithUser("user1", isDefault: true)
.WithGroup("test-group")
.WithProject("test-project", id: 1, @namespace: "test-group")
.BuildServer();

var client = server.CreateClient();

Assert.Multiple(() =>
{
Assert.That(getClient(client, 1), Is.Not.Null);
Assert.That(getClient(client, "test-group/test-project"), Is.Not.Null);
});
}

[TestCaseSource(nameof(GroupClientTestCases))]
public void Test_can_get_group_client(Func<IGitLabClient, GroupId, object> getClient)
{
using var server = new GitLabConfig()
.WithUser("user1", isDefault: true)
.WithGroup("test-group", group =>
{
group.Namespace = "parent-group";
group.Id = 1;
})
.BuildServer();

var client = server.CreateClient();

Assert.Multiple(() =>
{
Assert.That(getClient(client, 1), Is.Not.Null);
Assert.That(getClient(client, "parent-group/test-group"), Is.Not.Null);
});
}

[Test]
public void Test_getting_MergeRequestClient_for_group_is_not_implemented()
{
// GetGroupMergeRequest() is not implemented in the mock MergeRequestClient and will throw a NotImplementedException
// For that reason, this method is excluded from the test cases retruend by GroupClientTestCases
//
// This test checkes that this assumption still holds true and the method is rightly is skipped in GroupClientTestCases
// When a mock for GetGroupMergeRequest(), this test will fail.
// In this case, this test special logic for GetGroupMergeRequest() in GroupClientTestCases can be removed

using var server = new GitLabConfig()
.WithUser("user1", isDefault: true)
.WithGroup("test-group", group =>
{
group.Namespace = "parent-group";
group.Id = 1;
})
.BuildServer();

var client = server.CreateClient();

Assert.Multiple(() =>
{
Assert.Throws<NotImplementedException>(() => client.GetGroupMergeRequest(1));
Assert.Throws<NotImplementedException>(() => client.GetGroupMergeRequest("parent-group/test-group"));
});
}

private static IEnumerable<MethodInfo> GetMethods<TParameter>()
{
return typeof(IGitLabClient)
.GetMethods()
.Where(method => method.IsPublic)
.Where(method => method.GetParameters().Length == 1 && method.GetParameters()[0].ParameterType == typeof(TParameter));
}
}
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/ClusterClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class ClusterClient : ClientBase, IClusterClient
public ClusterClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public IEnumerable<ClusterInfo> All => throw new NotImplementedException();
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/CommitClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal sealed class CommitClient : ClientBase, ICommitClient
public CommitClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public Commit Create(CommitCreate commit)
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/CommitStatusClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal sealed class CommitStatusClient : ClientBase, ICommitStatusClient
public CommitStatusClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public CommitStatusCreate AddOrUpdate(CommitStatusCreate status)
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/EnvironmentClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ internal sealed class EnvironmentClient : ClientBase, IEnvironmentClient
public EnvironmentClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public IEnumerable<EnvironmentInfo> All => throw new NotImplementedException();
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/EventClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public EventClient(ClientContext context, int? userId = null, ProjectId? project
: base(context)
{
_userId = userId;
_projectId = projectId.HasValue ? Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id : null;
_projectId = projectId.HasValue ? Server.AllProjects.FindProject(projectId.ValueAsString()).Id : null;
}

IEnumerable<Models.Event> IEventClient.Get(EventQuery query)
Expand Down
5 changes: 1 addition & 4 deletions NGitLab.Mock/Clients/GitLabClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,7 @@ public IGraphQLClient GraphQL

public IProtectedBranchClient GetProtectedBranchClient(ProjectId projectId) => new ProtectedBranchClient(Context, projectId);

public IProtectedTagClient GetProtectedTagClient(ProjectId projectId)
{
throw new System.NotImplementedException();
}
public IProtectedTagClient GetProtectedTagClient(ProjectId projectId) => new ProtectedTagClient(Context, projectId);

public ISearchClient GetGroupSearchClient(int groupId) => GetGroupSearchClient((long)groupId);

Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/GroupBadgeClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class GroupBadgeClient : ClientBase, IGroupBadgeClient
public GroupBadgeClient(ClientContext context, GroupId groupId)
: base(context)
{
_groupId = Server.AllGroups.FindGroup(groupId.ValueAsUriParameter()).Id;
_groupId = Server.AllGroups.FindGroup(groupId.ValueAsString()).Id;
}

public Models.Badge this[int id]
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/GroupHooksClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class GroupHooksClient : ClientBase, IGroupHooksClient
public GroupHooksClient(ClientContext context, GroupId groupId)
: base(context)
{
_groupId = Server.AllGroups.FindGroup(groupId.ValueAsUriParameter()).Id;
_groupId = Server.AllGroups.FindGroup(groupId.ValueAsString()).Id;
}

public IEnumerable<Models.GroupHook> All
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/GroupSearchClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public GroupSearchClient(ClientContext context, GroupId groupId)
: base(context)
{
_context = context;
_groupId = Server.AllGroups.FindGroup(groupId.ValueAsUriParameter()).Id;
_groupId = Server.AllGroups.FindGroup(groupId.ValueAsString()).Id;
}

public GitLabCollectionResponse<SearchBlob> GetBlobsAsync(SearchQuery query)
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/GroupVariableClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class GroupVariableClient : ClientBase, IGroupVariableClient
public GroupVariableClient(ClientContext context, GroupId groupId)
: base(context)
{
_groupId = Server.AllGroups.FindGroup(groupId.ValueAsUriParameter()).Id;
_groupId = Server.AllGroups.FindGroup(groupId.ValueAsString()).Id;
}

public Variable this[string key] => throw new NotImplementedException();
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/JobClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal sealed class JobClient : ClientBase, IJobClient
public JobClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public Models.Job Get(int jobId)
Expand Down
5 changes: 4 additions & 1 deletion NGitLab.Mock/Clients/MergeRequestClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@ public MergeRequestClient(ClientContext context)
public MergeRequestClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public MergeRequestClient(ClientContext context, GroupId groupId)
: base(context)
{
// Support for group-level Merge Requests is not implemented in the mocks yet.
// For this reason, the GetGroupMergeRequest() method is excluded from the test cases in GitLabClientMockTest (see GroupClientTestCases).
// The exclusion in the test should be removed when support for support for merge requests in groups is implemented.
throw new NotImplementedException();
}

Expand Down
4 changes: 2 additions & 2 deletions NGitLab.Mock/Clients/MilestoneClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ public MilestoneClient(ClientContext context, IIdOrPathAddressable id, Milestone
{
_resourceId = scope switch
{
MilestoneScope.Groups => Server.AllGroups.FindGroup(id.ValueAsUriParameter()).Id,
MilestoneScope.Projects => Server.AllProjects.FindProject(id.ValueAsUriParameter()).Id,
MilestoneScope.Groups => Server.AllGroups.FindGroup(id.ValueAsString()).Id,
MilestoneScope.Projects => Server.AllProjects.FindProject(id.ValueAsString()).Id,
_ => throw new NotSupportedException($"{scope} milestone is not supported yet."),
};
Scope = scope;
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/PipelineClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public PipelineClient(ClientContext context, IJobClient jobClient, ProjectId pro
: base(context)
{
_jobClient = jobClient;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public Models.Pipeline this[int id]
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/ProjectBadgeClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal sealed class ProjectBadgeClient : ClientBase, IProjectBadgeClient
public ProjectBadgeClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public Models.Badge this[int id]
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/ProjectIssueNoteClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class ProjectIssueNoteClient : ClientBase, IProjectIssueNoteClie
public ProjectIssueNoteClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public Models.ProjectIssueNote Create(ProjectIssueNoteCreate create)
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/ProjectLevelApprovalRulesClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class ProjectLevelApprovalRulesClient : ClientBase, IProjectLeve
public ProjectLevelApprovalRulesClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public List<ApprovalRule> GetProjectLevelApprovalRules()
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/ProjectSearchClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public ProjectSearchClient(ClientContext context, ProjectId projectId)
: base(context)
{
_context = context;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public GitLabCollectionResponse<SearchBlob> GetBlobsAsync(SearchQuery query)
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/ProjectVariableClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class ProjectVariableClient : ClientBase, IProjectVariableClient
public ProjectVariableClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public Variable this[string key] => throw new NotImplementedException();
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/ProtectedBranchClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class ProtectedBranchClient : ClientBase, IProtectedBranchClient
public ProtectedBranchClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public Models.ProtectedBranch GetProtectedBranch(string branchName)
Expand Down
57 changes: 57 additions & 0 deletions NGitLab.Mock/Clients/ProtectedTagClient.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using NGitLab.Models;

namespace NGitLab.Mock.Clients;

internal sealed class ProtectedTagClient : ClientBase, IProtectedTagClient
{
private readonly int _projectId;

public ProtectedTagClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public ProtectedTag GetProtectedTag(string name)
{
throw new System.NotImplementedException();
}

public Task<ProtectedTag> GetProtectedTagAsync(string name, CancellationToken cancellationToken = default)
{
throw new System.NotImplementedException();
}

public IEnumerable<ProtectedTag> GetProtectedTags()
{
throw new System.NotImplementedException();
}

public GitLabCollectionResponse<ProtectedTag> GetProtectedTagsAsync()
{
throw new System.NotImplementedException();
}

public ProtectedTag ProtectTag(TagProtect protect)
{
throw new System.NotImplementedException();
}

public Task<ProtectedTag> ProtectTagAsync(TagProtect protect, CancellationToken cancellationToken = default)
{
throw new System.NotImplementedException();
}

public void UnprotectTag(string name)
{
throw new System.NotImplementedException();
}

public Task UnprotectTagAsync(string name, CancellationToken cancellationToken = default)
{
throw new System.NotImplementedException();
}
}
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/ReleaseClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal sealed class ReleaseClient : ClientBase, IReleaseClient
public ReleaseClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public IEnumerable<Models.ReleaseInfo> All
Expand Down
2 changes: 1 addition & 1 deletion NGitLab.Mock/Clients/RepositoryClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ internal sealed class RepositoryClient : ClientBase, IRepositoryClient
public RepositoryClient(ClientContext context, ProjectId projectId)
: base(context)
{
_projectId = Server.AllProjects.FindProject(projectId.ValueAsUriParameter()).Id;
_projectId = Server.AllProjects.FindProject(projectId.ValueAsString()).Id;
}

public ITagClient Tags => new TagClient(Context, _projectId);
Expand Down
Loading

0 comments on commit bbabe0e

Please sign in to comment.