Skip to content

Commit

Permalink
removed warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
petrsvihlik committed Feb 24, 2024
1 parent 6984a83 commit 0a6aeea
Show file tree
Hide file tree
Showing 45 changed files with 243 additions and 412 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/integrate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- name: Setup .NET
uses: actions/setup-dotnet@v3
with:
dotnet-version: '7.0.x'
dotnet-version: '8.0.x'
- name: Restore dependencies
run: dotnet restore -s ${MYGET_FEED} -s https://api.nuget.org/v3/index.json
env:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
- name: Setup .NET
uses: actions/setup-dotnet@v3
with:
dotnet-version: '7.0.x'
dotnet-version: '8.0.x'
- name: Restore dependencies
run: dotnet restore -s ${MYGET_FEED} -s https://api.nuget.org/v3/index.json
env:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- name: Setup .NET
uses: actions/setup-dotnet@v3
with:
dotnet-version: '7.0.x'
dotnet-version: '8.0.x'
- name: Extract version from tag
id: get_version
uses: battila7/get-version-action@v2
Expand Down
17 changes: 6 additions & 11 deletions src/WopiHost.Abstractions/WopiAuthorizationRequirement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,14 @@ namespace WopiHost.Abstractions;
/// <summary>
/// Represents an authorization requirement for a given combination of resource, user, and action.
/// </summary>
public class WopiAuthorizationRequirement : IAuthorizationRequirement
/// <remarks>
/// Creates an instance of <see cref="WopiAuthorizationRequirement"/> initialized with <paramref name="permission"/>.
/// </remarks>
/// <param name="permission">Permissions required for a given combination of resource, user, and action.</param>
public class WopiAuthorizationRequirement(Permission permission) : IAuthorizationRequirement

Check warning on line 12 in src/WopiHost.Abstractions/WopiAuthorizationRequirement.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Abstractions/WopiAuthorizationRequirement.cs#L12

Added line #L12 was not covered by tests
{
/// <summary>
/// Gets a permissions required for a given combination of resource, user, and action.
/// </summary>
public Permission Permission { get; }

/// <summary>
/// Creates an instance of <see cref="WopiAuthorizationRequirement"/> initialized with <paramref name="permission"/>.
/// </summary>
/// <param name="permission">Permissions required for a given combination of resource, user, and action.</param>
public WopiAuthorizationRequirement(Permission permission)
{
Permission = permission;
}
public Permission Permission { get; } = permission;

Check warning on line 17 in src/WopiHost.Abstractions/WopiAuthorizationRequirement.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Abstractions/WopiAuthorizationRequirement.cs#L17

Added line #L17 was not covered by tests
}
4 changes: 2 additions & 2 deletions src/WopiHost.Abstractions/WopiHost.Abstractions.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Authorization" Version="7.0.0" />
<PackageReference Include="Microsoft.AspNetCore.Authorization" Version="8.0.2" />
<PackageReference Include="System.Security.Claims" Version="4.3.0" />
<PackageReference Include="Microsoft.IdentityModel.Tokens" Version="6.35.0" />
<PackageReference Include="Microsoft.IdentityModel.Tokens" Version="7.3.1" />
</ItemGroup>

</Project>
19 changes: 4 additions & 15 deletions src/WopiHost.Cobalt/CobaltHostLockingStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,9 @@

namespace WopiHost.Cobalt;

public class CobaltHostLockingStore : HostLockingStore
public class CobaltHostLockingStore(ClaimsPrincipal principal) : HostLockingStore
{
private readonly ClaimsPrincipal _principal;

public CobaltHostLockingStore(ClaimsPrincipal principal)
{
_principal = principal;
}
private readonly ClaimsPrincipal _principal = principal;

public override WhoAmIRequest.OutputType HandleWhoAmI(WhoAmIRequest.InputType input)
{
Expand Down Expand Up @@ -180,10 +175,7 @@ public override GetCoauthoringStatusRequest.OutputType HandleGetCoauthoringStatu
return result;
}

public override Dictionary<string, EditorsTableEntry> QueryEditorsTable()
{
return new Dictionary<string, EditorsTableEntry>();
}
public override Dictionary<string, EditorsTableEntry> QueryEditorsTable() => new Dictionary<string, EditorsTableEntry>();

public override JoinEditingSessionRequest.OutputType HandleJoinEditingSession(JoinEditingSessionRequest.InputType input)
{
Expand Down Expand Up @@ -220,10 +212,7 @@ public override RemoveEditorMetadataRequest.OutputType HandleRemoveEditorMetadat
return result;
}

public override ulong GetEditorsTableWaterline()
{
return 0;
}
public override ulong GetEditorsTableWaterline() => 0;

public override AmIAloneRequest.OutputType HandleAmIAlone(AmIAloneRequest.InputType input)
{
Expand Down
17 changes: 7 additions & 10 deletions src/WopiHost.Core/Controllers/ContainersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,15 @@ namespace WopiHost.Core.Controllers;
/// <summary>
/// Implementation of WOPI server protocol https://msdn.microsoft.com/en-us/library/hh659001.aspx
/// </summary>
/// <remarks>
/// Creates an instance of <see cref="ContainersController"/>.
/// </remarks>
/// <param name="storageProvider">Storage provider instance for retrieving files and folders.</param>
/// <param name="securityHandler">Security handler instance for performing security-related operations.</param>
/// <param name="wopiHostOptions">WOPI Host configuration</param>
[Route("wopi/[controller]")]
public class ContainersController : WopiControllerBase
public class ContainersController(IOptionsSnapshot<WopiHostOptions> wopiHostOptions, IWopiStorageProvider storageProvider, IWopiSecurityHandler securityHandler) : WopiControllerBase(storageProvider, securityHandler, wopiHostOptions)

Check warning on line 20 in src/WopiHost.Core/Controllers/ContainersController.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/ContainersController.cs#L20

Added line #L20 was not covered by tests
{
/// <summary>
/// Creates an instance of <see cref="ContainersController"/>.
/// </summary>
/// <param name="storageProvider">Storage provider instance for retrieving files and folders.</param>
/// <param name="securityHandler">Security handler instance for performing security-related operations.</param>
/// <param name="wopiHostOptions">WOPI Host configuration</param>
public ContainersController(IOptionsSnapshot<WopiHostOptions> wopiHostOptions, IWopiStorageProvider storageProvider, IWopiSecurityHandler securityHandler) : base(storageProvider, securityHandler, wopiHostOptions)
{
}

/// <summary>
/// Returns the metadata about a container specified by an identifier.
Expand Down
32 changes: 14 additions & 18 deletions src/WopiHost.Core/Controllers/EcosystemController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,23 @@ namespace WopiHost.Core.Controllers;
/// <summary>
/// Implementation of WOPI server protocol http://wopi.readthedocs.io/projects/wopirest/en/latest/ecosystem/CheckEcosystem.html
/// </summary>
/// <remarks>
/// Creates an instance of <see cref="EcosystemController"/>.
/// </remarks>
/// <param name="storageProvider">Storage provider instance for retrieving files and folders.</param>
/// <param name="securityHandler">Security handler instance for performing security-related operations.</param>
/// <param name="wopiHostOptions">WOPI Host configuration</param>
[Route("wopi/[controller]")]
public class EcosystemController : WopiControllerBase
public class EcosystemController(IWopiStorageProvider storageProvider, IWopiSecurityHandler securityHandler, IOptionsSnapshot<WopiHostOptions> wopiHostOptions) : WopiControllerBase(storageProvider, securityHandler, wopiHostOptions)

Check warning on line 19 in src/WopiHost.Core/Controllers/EcosystemController.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/EcosystemController.cs#L19

Added line #L19 was not covered by tests
{
/// <summary>
/// Creates an instance of <see cref="EcosystemController"/>.
/// </summary>
/// <param name="storageProvider">Storage provider instance for retrieving files and folders.</param>
/// <param name="securityHandler">Security handler instance for performing security-related operations.</param>
/// <param name="wopiHostOptions">WOPI Host configuration</param>
public EcosystemController(IWopiStorageProvider storageProvider, IWopiSecurityHandler securityHandler, IOptionsSnapshot<WopiHostOptions> wopiHostOptions)
: base(storageProvider, securityHandler, wopiHostOptions)
{
}

/// <summary>
/// The GetRootContainer operation returns the root container. A WOPI client can use this operation to get a reference to the root container, from which the client can call EnumerateChildren (containers) to navigate a container hierarchy.
/// Specification: http://wopi.readthedocs.io/projects/wopirest/en/latest/ecosystem/GetRootContainer.html
/// Example URL: GET /wopi/ecosystem/root_container_pointer
/// </summary>
/// <returns></returns>
[HttpGet("root_container_pointer")]
/// <summary>
/// The GetRootContainer operation returns the root container. A WOPI client can use this operation to get a reference to the root container, from which the client can call EnumerateChildren (containers) to navigate a container hierarchy.
/// Specification: http://wopi.readthedocs.io/projects/wopirest/en/latest/ecosystem/GetRootContainer.html
/// Example URL: GET /wopi/ecosystem/root_container_pointer
/// </summary>
/// <returns></returns>
[HttpGet("root_container_pointer")]
[Produces(MediaTypeNames.Application.Json)]
public RootContainerInfo GetRootContainer() //TODO: fix the path
{
Expand Down
12 changes: 4 additions & 8 deletions src/WopiHost.Core/Controllers/FilesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,9 @@ public async Task<IActionResult> PutFile(string id)
/// </summary>
/// <param name="id">File identifier.</param>
/// <returns>Returns <see cref="StatusCodes.Status200OK"/> if succeeded.</returns>
[HttpPost("{id}"), WopiOverrideHeader(new[] { "PUT_RELATIVE" })]
[HttpPost("{id}"), WopiOverrideHeader(["PUT_RELATIVE"])]
#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously
public async Task<IActionResult> PutRelativeFile(string id)
#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously
{
throw new NotImplementedException($"{nameof(PutRelativeFile)} is not implemented yet.");
}
public async Task<IActionResult> PutRelativeFile(string id) => throw new NotImplementedException($"{nameof(PutRelativeFile)} is not implemented yet.");

Check warning on line 160 in src/WopiHost.Core/Controllers/FilesController.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/FilesController.cs#L160

Added line #L160 was not covered by tests

/// <summary>
/// Changes the contents of the file in accordance with [MS-FSSHTTP].
Expand All @@ -170,7 +166,7 @@ public async Task<IActionResult> PutRelativeFile(string id)
/// Example URL path: /wopi/files/(file_id)
/// </summary>
/// <param name="id">File identifier.</param>
[HttpPost("{id}"), WopiOverrideHeader(new[] { "COBALT" })]
[HttpPost("{id}"), WopiOverrideHeader(["COBALT"])]
public async Task<IActionResult> ProcessCobalt(string id)
{
// Check permissions
Expand Down Expand Up @@ -202,7 +198,7 @@ public async Task<IActionResult> ProcessCobalt(string id)
/// Example URL path: /wopi/files/(file_id)
/// </summary>
/// <param name="id">File identifier.</param>
[HttpPost("{id}"), WopiOverrideHeader(new[] { "LOCK", "UNLOCK", "REFRESH_LOCK", "GET_LOCK" })]
[HttpPost("{id}"), WopiOverrideHeader(["LOCK", "UNLOCK", "REFRESH_LOCK", "GET_LOCK"])]
public IActionResult ProcessLock(string id)
{
string oldLock = Request.Headers[WopiHeaders.OLD_LOCK];
Expand Down
32 changes: 13 additions & 19 deletions src/WopiHost.Core/Controllers/WopiBootstrapperController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,15 @@ namespace WopiHost.Core.Controllers;
/// <summary>
/// Controller containing the bootstrap operation.
/// </summary>
/// <remarks>
/// Creates an instance of <see cref="WopiBootstrapperController"/>.
/// </remarks>
/// <param name="storageProvider">Storage provider instance for retrieving files and folders.</param>
/// <param name="securityHandler">Security handler instance for performing security-related operations.</param>
/// <param name="wopiHostOptions">WOPI Host configuration</param>
[Route("wopibootstrapper")]
public class WopiBootstrapperController : WopiControllerBase
public class WopiBootstrapperController(IWopiStorageProvider storageProvider, IWopiSecurityHandler securityHandler, IOptionsSnapshot<WopiHostOptions> wopiHostOptions) : WopiControllerBase(storageProvider, securityHandler, wopiHostOptions)

Check warning on line 22 in src/WopiHost.Core/Controllers/WopiBootstrapperController.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/WopiBootstrapperController.cs#L22

Added line #L22 was not covered by tests
{
/// <summary>
/// Creates an instance of <see cref="WopiBootstrapperController"/>.
/// </summary>
/// <param name="storageProvider">Storage provider instance for retrieving files and folders.</param>
/// <param name="securityHandler">Security handler instance for performing security-related operations.</param>
/// <param name="wopiHostOptions">WOPI Host configuration</param>
public WopiBootstrapperController(IWopiStorageProvider storageProvider, IWopiSecurityHandler securityHandler, IOptionsSnapshot<WopiHostOptions> wopiHostOptions)
: base(storageProvider, securityHandler, wopiHostOptions)
{
}

/// <summary>
/// Gets information about the root container.
Expand All @@ -34,7 +30,7 @@ public WopiBootstrapperController(IWopiStorageProvider storageProvider, IWopiSec
[Produces(MediaTypeNames.Application.Json)]
public IActionResult GetRootContainer() //TODO: fix the path
{
var authorizationHeader = HttpContext.Request.Headers["Authorization"];
var authorizationHeader = HttpContext.Request.Headers.Authorization;

Check warning on line 33 in src/WopiHost.Core/Controllers/WopiBootstrapperController.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/WopiBootstrapperController.cs#L33

Added line #L33 was not covered by tests
var ecosystemOperation = HttpContext.Request.Headers[WopiHeaders.ECOSYSTEM_OPERATION];
var wopiSrc = HttpContext.Request.Headers[WopiHeaders.WOPI_SRC].FirstOrDefault();

Expand Down Expand Up @@ -98,20 +94,18 @@ public IActionResult GetRootContainer() //TODO: fix the path

private string GetIdFromUrl(string resourceUrl)
{
var resourceId = resourceUrl[(resourceUrl.LastIndexOf("/", StringComparison.Ordinal) + 1)..];
var queryIndex = resourceId.IndexOf("?", StringComparison.Ordinal);
var resourceId = resourceUrl[(resourceUrl.LastIndexOf('/') + 1)..];
var queryIndex = resourceId.IndexOf('?');

Check warning on line 98 in src/WopiHost.Core/Controllers/WopiBootstrapperController.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/WopiBootstrapperController.cs#L97-L98

Added lines #L97 - L98 were not covered by tests
if (queryIndex > -1)
{
resourceId = resourceId.Substring(0, queryIndex);
resourceId = resourceId[..queryIndex];

Check warning on line 101 in src/WopiHost.Core/Controllers/WopiBootstrapperController.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/WopiBootstrapperController.cs#L101

Added line #L101 was not covered by tests
}
resourceId = Uri.UnescapeDataString(resourceId);
return resourceId;
}

private bool ValidateAuthorizationHeader(StringValues authorizationHeader)
{
private bool ValidateAuthorizationHeader(StringValues authorizationHeader) =>
//TODO: implement header validation http://wopi.readthedocs.io/projects/wopirest/en/latest/bootstrapper/GetRootContainer.html#sample-response
// http://stackoverflow.com/questions/31948426/oauth-bearer-token-authentication-is-not-passing-signature-validation
return true;
}
true;

Check warning on line 110 in src/WopiHost.Core/Controllers/WopiBootstrapperController.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/WopiBootstrapperController.cs#L110

Added line #L110 was not covered by tests
}
27 changes: 10 additions & 17 deletions src/WopiHost.Core/Controllers/WopiControllerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,28 @@ namespace WopiHost.Core.Controllers;
/// <summary>
/// Extends the <see cref="ControllerBase"/> with some basic WOPI-related functionality.
/// </summary>
public abstract class WopiControllerBase : ControllerBase
/// <remarks>
/// Default constructor.
/// </remarks>
/// <param name="storageProvider">Object facilitating access to the storage of WOPI files.</param>
/// <param name="securityHandler">Object facilitating security-related actions.</param>
/// <param name="wopiHostOptions">WOPI Host configuration object</param>
public abstract class WopiControllerBase(IWopiStorageProvider storageProvider, IWopiSecurityHandler securityHandler, IOptionsSnapshot<WopiHostOptions> wopiHostOptions) : ControllerBase

Check warning on line 19 in src/WopiHost.Core/Controllers/WopiControllerBase.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/WopiControllerBase.cs#L19

Added line #L19 was not covered by tests
{
/// <summary>
/// Provides access to the storage.
/// </summary>
protected IWopiStorageProvider StorageProvider { get; }
protected IWopiStorageProvider StorageProvider { get; } = storageProvider;

Check warning on line 24 in src/WopiHost.Core/Controllers/WopiControllerBase.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/WopiControllerBase.cs#L24

Added line #L24 was not covered by tests

/// <summary>
/// Provides security-related actions.
/// </summary>
protected IWopiSecurityHandler SecurityHandler { get; }
protected IWopiSecurityHandler SecurityHandler { get; } = securityHandler;

Check warning on line 29 in src/WopiHost.Core/Controllers/WopiControllerBase.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/WopiControllerBase.cs#L29

Added line #L29 was not covered by tests

/// <summary>
/// WOPI Host configuration object.
/// </summary>
protected IOptionsSnapshot<WopiHostOptions> WopiHostOptions { get; }
protected IOptionsSnapshot<WopiHostOptions> WopiHostOptions { get; } = wopiHostOptions;

Check warning on line 34 in src/WopiHost.Core/Controllers/WopiControllerBase.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/Controllers/WopiControllerBase.cs#L34

Added line #L34 was not covered by tests

/// <summary>
/// WOPI Host base URL
Expand All @@ -45,19 +51,6 @@ protected string AccessToken
}
}

/// <summary>
/// Default constructor.
/// </summary>
/// <param name="storageProvider">Object facilitating access to the storage of WOPI files.</param>
/// <param name="securityHandler">Object facilitating security-related actions.</param>
/// <param name="wopiHostOptions">WOPI Host configuration object</param>
protected WopiControllerBase(IWopiStorageProvider storageProvider, IWopiSecurityHandler securityHandler, IOptionsSnapshot<WopiHostOptions> wopiHostOptions)
{
StorageProvider = storageProvider;
SecurityHandler = securityHandler;
WopiHostOptions = wopiHostOptions;
}

/// <summary>
/// Creates a simple URL to access a WOPI object of choice.
/// </summary>
Expand Down
10 changes: 2 additions & 8 deletions src/WopiHost.Core/FileExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,9 @@ public static class FileExtensions
/// <returns>CheckFileInfo model</returns>
public static CheckFileInfo GetCheckFileInfo(this IWopiFile file, ClaimsPrincipal principal, HostCapabilities capabilities)
{
if (file is null)
{
throw new ArgumentNullException(nameof(file));
}
ArgumentNullException.ThrowIfNull(file);

Check warning on line 25 in src/WopiHost.Core/FileExtensions.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/FileExtensions.cs#L25

Added line #L25 was not covered by tests

if (capabilities is null)
{
throw new ArgumentNullException(nameof(capabilities));
}
ArgumentNullException.ThrowIfNull(capabilities);

Check warning on line 27 in src/WopiHost.Core/FileExtensions.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/FileExtensions.cs#L27

Added line #L27 was not covered by tests

var checkFileInfo = new CheckFileInfo();
if (principal is not null)
Expand Down
27 changes: 9 additions & 18 deletions src/WopiHost.Core/HttpHeaderAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,20 @@ namespace WopiHost.Core;
/// <summary>
/// A header-based constraint for HTTP actions.
/// </summary>
/// <remarks>
/// Creates an instance of a constraint based on a header name and allowed values.
/// </remarks>
/// <param name="header">Header name to check.</param>
/// <param name="values">Accepted header values.</param>
[AttributeUsage(AttributeTargets.Method)]
public class HttpHeaderAttribute : Attribute, IActionConstraint
public class HttpHeaderAttribute(string header, params string[] values) : Attribute, IActionConstraint

Check warning on line 14 in src/WopiHost.Core/HttpHeaderAttribute.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/HttpHeaderAttribute.cs#L14

Added line #L14 was not covered by tests
{
private string Header { get; set; }
private string Header { get; set; } = header;

Check warning on line 16 in src/WopiHost.Core/HttpHeaderAttribute.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/HttpHeaderAttribute.cs#L16

Added line #L16 was not covered by tests

private string[] Values { get; set; }

/// <summary>
/// Creates an instance of a constraint based on a header name and allowed values.
/// </summary>
/// <param name="header">Header name to check.</param>
/// <param name="values">Accepted header values.</param>
public HttpHeaderAttribute(string header, params string[] values)
{
Header = header;
Values = values;
}
private string[] Values { get; set; } = values;

Check warning on line 18 in src/WopiHost.Core/HttpHeaderAttribute.cs

View check run for this annotation

Codecov / codecov/patch

src/WopiHost.Core/HttpHeaderAttribute.cs#L18

Added line #L18 was not covered by tests

/// <inheritdoc />
public bool Accept(ActionConstraintContext context)
{
return (context is not null) && context.RouteContext.HttpContext.Request.Headers.TryGetValue(Header, out var value) && Values.Contains(value[0]);
}
public bool Accept(ActionConstraintContext context) => (context is not null) && context.RouteContext.HttpContext.Request.Headers.TryGetValue(Header, out var value) && Values.Contains(value[0]);

/// <inheritdoc />
public int Order => 0;
Expand Down
Loading

0 comments on commit 0a6aeea

Please sign in to comment.