Skip to content

feat: add IWorkPackages.UpdateAsync; tidy the handler and read-only tests #58

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.

Sign up for GitHub

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

Merged
davidnmbond merged 1 commit into from
Nov 9, 2026
Merged

feat: add IWorkPackages.UpdateAsync; tidy the handler and read-only tests #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 18 additions & 22 deletions OpenProject.Api.Test/Tests/ReadOnlyOptionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using OpenProject.Api.Data.Models.Create;
using OpenProject.Api.Data.Models.Update;

namespace OpenProject.Api.Test.Tests;

Expand All @@ -23,36 +24,31 @@ public void IsReadOnly_DefaultsToFalse()
=> new OpenProjectClientOptions { Uri = new Uri("http://127.0.0.1:1"), ApiKey = "k" }
.IsReadOnly.Should().BeFalse();

[Fact]
public async Task ReadOnly_Create_ThrowsInvalidOperationBeforeSending()
private static async Task<bool> WasRefusedAsync<T>(bool isReadOnly, Func<OpenProjectClient, Task<T>> call)
{
using var client = ClientFor(isReadOnly: true);
using var client = ClientFor(isReadOnly);

var act = () => client.Projects.CreateAsync(new ProjectCreate { Name = "p" }, CancellationToken.None);

var thrown = await act.Should().ThrowAsync<Exception>();
IsRefused(thrown.Which).Should().BeTrue();
var thrown = await FluentActions.Awaiting(() => call(client)).Should().ThrowAsync<Exception>();
return IsRefused(thrown.Which);
}

[Fact]
public async Task NotReadOnly_Create_IsAttemptedAndFailsOnConnectionNotOnPolicy()
{
using var client = ClientFor(isReadOnly: false);
public async Task ReadOnly_Create_ThrowsInvalidOperationBeforeSending()
=> (await WasRefusedAsync(true, c => c.Projects.CreateAsync(new ProjectCreate { Name = "p" }, CancellationToken.None)))
.Should().BeTrue();

var act = () => client.Projects.CreateAsync(new ProjectCreate { Name = "p" }, CancellationToken.None);
[Fact]
public async Task ReadOnly_WorkPackageUpdate_ThrowsInvalidOperationBeforeSending()
=> (await WasRefusedAsync(true, c => c.WorkPackages.UpdateAsync(1, new WorkPackageUpdate { LockVersion = 1 }, CancellationToken.None)))
.Should().BeTrue();

var thrown = await act.Should().ThrowAsync<Exception>();
IsRefused(thrown.Which).Should().BeFalse();
}
[Fact]
public async Task NotReadOnly_Create_IsAttemptedAndFailsOnConnectionNotOnPolicy()
=> (await WasRefusedAsync(false, c => c.Projects.CreateAsync(new ProjectCreate { Name = "p" }, CancellationToken.None)))
.Should().BeFalse();

[Fact]
public async Task ReadOnly_Get_IsNotRefused()
{
using var client = ClientFor(isReadOnly: true);

var act = () => client.Projects.GetAllAsync(CancellationToken.None);

var thrown = await act.Should().ThrowAsync<Exception>();
IsRefused(thrown.Which).Should().BeFalse();
}
=> (await WasRefusedAsync(true, c => c.Projects.GetAllAsync(CancellationToken.None)))
.Should().BeFalse();
}
22 changes: 22 additions & 0 deletions OpenProject.Api.Test/Tests/SortByTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
using OpenProject.Api.Queries;

namespace OpenProject.Api.Test.Tests;

/// <summary>
/// <see cref="SortBy"/> renders the sortBy query syntax; no configuration, network or server needed.
/// </summary>
public class SortByTests
{
[Fact]
public void ToString_RendersEachFieldAndDirectionAsAJsonPair()
=> new SortBy(
[
new FieldAndDirection { Field = "id", Direction = Direction.Ascending },
new FieldAndDirection { Field = "updatedAt", Direction = Direction.Descending }
])
.ToString().Should().Be("""[["id","asc"],["updatedAt","desc"]]""");

[Fact]
public void ToString_OfNoFields_IsAnEmptyArray()
=> new SortBy([]).ToString().Should().Be("[]");
}
42 changes: 24 additions & 18 deletions OpenProject.Api/AuthenticatedHttpClientHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,15 @@ protected override async Task<HttpResponseMessage> SendAsync(
HttpRequestMessage request,
CancellationToken cancellationToken)
{
if (_options.IsReadOnly
&& request.Method != HttpMethod.Get
&& request.Method != HttpMethod.Head
&& request.Method != HttpMethod.Options)
if (_options.IsReadOnly && !IsRead(request.Method))
{
throw new InvalidOperationException($"The client is read-only; {request.Method} {request.RequestUri?.AbsolutePath} would change the server.");
}

// Set basic auth header
request.Headers.Authorization = new AuthenticationHeaderValue("Basic", Convert.ToBase64String(Encoding.ASCII.GetBytes($"apikey:{_options.ApiKey}")));

// Get a GUID to uniquely identify the request
var guid = Guid.NewGuid();
if (_logger?.IsEnabled(LogLevel.Debug) == true)
{
_logger.LogDebug("{Guid}:{RequestMethod}:{RequestUri}\nHeaders:{Headers}\nBody:{Body}",
guid.ToString(),
request.Method,
request.RequestUri,
request.Headers,
request.Content is null ? null : await request.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false)
);
}
await LogRequestAsync(guid, request, cancellationToken).ConfigureAwait(false);

HttpResponseMessage response;
try
Expand All @@ -71,15 +57,35 @@ protected override async Task<HttpResponseMessage> SendAsync(
throw;
}

await LogResponseAsync(guid, request, response, cancellationToken).ConfigureAwait(false);
return response;
}

private static bool IsRead(HttpMethod method)
=> method == HttpMethod.Get || method == HttpMethod.Head || method == HttpMethod.Options;

private async Task LogRequestAsync(Guid guid, HttpRequestMessage request, CancellationToken cancellationToken)
{
if (_logger?.IsEnabled(LogLevel.Debug) == true)
{
_logger.LogDebug("{Guid}:{RequestMethod}:{RequestUri}\nHeaders:{Headers}\nBody:{Body}",
guid.ToString(),
request.Method,
request.RequestUri,
request.Headers,
request.Content is null ? null : await request.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false));
}
}

private async Task LogResponseAsync(Guid guid, HttpRequestMessage request, HttpResponseMessage response, CancellationToken cancellationToken)
{
if (_logger?.IsEnabled(LogLevel.Debug) == true)
{
_logger.LogDebug("{Guid}:{ResponseStatusCode}:{Body}",
guid.ToString(),
response.StatusCode,
request.Content is null ? null : await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false));

@codacy-production codacy-production Bot Nov 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM RISK

Check response.Content before reading and logging the response body. Using request.Content here prevents response bodies from being logged for typical GET requests.

TIP React 👍 if this review is right, or reply with what's wrong and how it could be better to help improve the reviewer.

}

return response;
}

/// <summary>
Expand Down
13 changes: 13 additions & 0 deletions OpenProject.Api/Interfaces/Controllers/IWorkPackages.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using OpenProject.Api.Data.Models;
using OpenProject.Api.Data.Models.Create;
using OpenProject.Api.Data.Models.Update;
using OpenProject.Api.Queries;

namespace OpenProject.Api.Interfaces.Controllers;
Expand Down Expand Up @@ -53,6 +54,18 @@ Task<WorkPackage> CreateAsync(
[Body] WorkPackageCreate entity,
CancellationToken cancellationToken);

/// <summary>
/// Update a Work Package by ID and return the updated entity
/// </summary>
/// <param name="id">The work package ID.</param>
/// <param name="entity">The fields to change; LockVersion must match the current version.</param>
/// <param name="cancellationToken">The cancellation token.</param>
[Patch("/work_packages/{id}")]
Task<WorkPackage> UpdateAsync(
int id,
[Body] WorkPackageUpdate entity,
CancellationToken cancellationToken);

/// <summary>
/// Delete a Work Package by ID
/// </summary>
Expand Down
Loading