Skip to content

Commit

Permalink
Further ETag testing and changes.
Browse files Browse the repository at this point in the history
  • Loading branch information
chullybun committed Mar 4, 2024
1 parent bd74f3a commit 448792c
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 35 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Represents the **NuGet** versions.
- *Fixed*: Validation extensions `Exists` and `ExistsAsync` which expect a non-null resultant value have been renamed to `ValueExists` and `ValueExistsAsync` to improve usability; also they are `IResult` aware and will act accordingly.
- *Fixed*: The `ETag` HTTP handling has been updated to correctly output and expect the weak `W/"xxxx"` format.
- *Fixed*: The `ETagGenerator` implementation has been further optimized to minimize unneccessary string allocations.
- *Fixed*: The `ValueContentResult` will only generate a response header ETag (`ETagGenerator`) for a `GET` or `HEAD` request. The underlying result `IETag.ETag` is used as-is where there is no query string; otherwise, generates as assumes query string will alter result (i.e. filtering, paging, sorting, etc.). The result `IETag.ETag` is unchanged so the consumer can still use as required for a further operation.
- *Fixed*: The `SettingsBase` has been optimized. The internal recursion checking has been removed and as such an endless loop (`StackOverflowException`) may occur where misconfigured; given frequency of `IConfiguration` usage the resulting performance is deemed more important. Additionally, `prefixes` are now optional.
- The existing support of referencing a settings property by name (`settings.GetValue<T>("NamedProperty")`) and it using reflection to find before querying the `IConfiguration` has been removed. This was not a common, or intended usage, and was somewhat magical, and finally was non-performant.

Expand Down
31 changes: 24 additions & 7 deletions src/CoreEx.AspNetCore/WebApis/ValueContentResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Net;
using System.Net.Mime;
Expand Down Expand Up @@ -149,22 +148,39 @@ public static bool TryCreateValueContentResult<T>(T value, HttpStatusCode status
throw new InvalidOperationException("Function has not returned a result; no AlternateStatusCode has been configured to return.");
}

// Where there is etag support and it is null (assumes auto-generation) then generate from the full value JSON contents as the baseline value.
var isTextSerializationEnabled = ExecutionContext.HasCurrent && ExecutionContext.Current.IsTextSerializationEnabled;
var etag = value is IETag vetag ? vetag.ETag : null;
if (etag is null)
{
if (isTextSerializationEnabled)
ExecutionContext.Current.IsTextSerializationEnabled = false;

etag = ETagGenerator.Generate(jsonSerializer, value);
if (value is IETag vetag2)
vetag2.ETag = etag;
}

// Where IncludeText is selected then enable before serialization occurs.
if (requestOptions.IncludeText && ExecutionContext.HasCurrent)
ExecutionContext.Current.IsTextSerializationEnabled = true;

// Serialize and generate the etag whilst also applying any filtering of the data where selected.
// Serialize the value performing any filtering as per the request options.
string? json = null;

if (requestOptions.IncludeFields != null && requestOptions.IncludeFields.Length > 0)
jsonSerializer.TryApplyFilter(val, requestOptions.IncludeFields, out json, JsonPropertyFilter.Include);
else if (requestOptions.ExcludeFields != null && requestOptions.ExcludeFields.Length > 0)
jsonSerializer.TryApplyFilter(val, requestOptions.ExcludeFields, out json, JsonPropertyFilter.Exclude);
else
json = jsonSerializer.Serialize(val);

// Generate the etag from the final JSON serialization and check for not-modified.
var result = GenerateETag(requestOptions, val, json, jsonSerializer);

// Reset the text serialization flag.
if (ExecutionContext.HasCurrent)
ExecutionContext.Current.IsTextSerializationEnabled = isTextSerializationEnabled;

// Check for not-modified and return status accordingly.
if (checkForNotModified && result.etag == requestOptions.ETag)
{
Expand All @@ -174,7 +190,7 @@ public static bool TryCreateValueContentResult<T>(T value, HttpStatusCode status
}

// Create and return the ValueContentResult.
primaryResult = new ValueContentResult(result.json!, statusCode, result.etag, paging, location);
primaryResult = new ValueContentResult(result.json!, statusCode, result.etag ?? etag, paging, location);
alternateResult = null;
return true;
}
Expand All @@ -189,15 +205,16 @@ public static bool TryCreateValueContentResult<T>(T value, HttpStatusCode status
/// <returns>The etag and serialized JSON (where performed).</returns>
internal static (string? etag, string? json) GenerateETag<T>(WebApiRequestOptions requestOptions, T value, string? json, IJsonSerializer jsonSerializer)
{
// Where not a GET or HEAD then no etag is generated; just use what we have.
if (!HttpMethods.IsGet(requestOptions.Request.Method) && !HttpMethods.IsHead(requestOptions.Request.Method))
return (value is IETag etag ? etag.ETag : null, json);

// Where no query string and there is an etag then that value should be leveraged as the fast-path.
if (!requestOptions.HasQueryString)
{
if (value is IETag etag && etag.ETag != null)
return (etag.ETag, json);

if (ExecutionContext.HasCurrent && ExecutionContext.Current.ResultETag != null)
return (ExecutionContext.Current.ResultETag, json);

// Where there is a collection then we need to generate a hash that represents the collection.
if (json is null && value is not string && value is IEnumerable coll)
{
Expand Down
23 changes: 18 additions & 5 deletions src/CoreEx.AspNetCore/WebApis/WebApi.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Avanade. Licensed under the MIT License. See https://github.com/Avanade/CoreEx

using CoreEx.Abstractions;
using CoreEx.AspNetCore.Http;
using CoreEx.Configuration;
using CoreEx.Entities;
Expand Down Expand Up @@ -710,17 +711,29 @@ private async Task<IActionResult> PutInternalAsync<TValue>(HttpRequest request,
/// </summary>
private ConcurrencyException? ConcurrencyETagMatching<TValue>(WebApiParam wap, TValue getValue, TValue putValue, bool autoConcurrency)
{
var et = putValue as IETag;
if (et != null || autoConcurrency)
var ptag = putValue as IETag;
if (ptag != null || autoConcurrency)
{
string? etag = et?.ETag ?? wap.RequestOptions.ETag;
string? etag = wap.RequestOptions.ETag ?? ptag?.ETag;
if (string.IsNullOrEmpty(etag))
return new ConcurrencyException($"An 'If-Match' header is required for an HTTP {wap.Request.Method} where the underlying entity supports concurrency (ETag).");

if (etag != null)
{
var getEt = ValueContentResult.GenerateETag(wap.RequestOptions, getValue!, null, JsonSerializer);
if (etag != getEt.etag)
var gtag = getValue is IETag getag ? getag.ETag : null;
if (gtag is null)
{
var isTextSerializationEnabled = ExecutionContext.HasCurrent && ExecutionContext.Current.IsTextSerializationEnabled;
if (isTextSerializationEnabled)
ExecutionContext.Current.IsTextSerializationEnabled = false;

gtag = ETagGenerator.Generate(JsonSerializer, getValue);

if (ExecutionContext.HasCurrent)
ExecutionContext.Current.IsTextSerializationEnabled = isTextSerializationEnabled;
}

if (etag != gtag)
return new ConcurrencyException();
}
}
Expand Down
10 changes: 8 additions & 2 deletions src/CoreEx.Database/DatabaseRecord.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

using CoreEx.Entities;
using System;
using System.Data;
using System.Data.Common;

namespace CoreEx.Database
Expand Down Expand Up @@ -36,7 +35,14 @@ public class DatabaseRecord(IDatabase database, DbDataReader dataReader)
/// </summary>
/// <param name="ordinal">The ordinal index.</param>
/// <returns>The value.</returns>
public object? GetValue(int ordinal) => DataReader.IsDBNull(ordinal) ? default! : DataReader.GetValue(ordinal);
public object? GetValue(int ordinal)
{
if (DataReader.IsDBNull(ordinal))
return default;

var val = DataReader.GetValue(ordinal);
return val is DateTime dt ? Cleaner.Clean(dt, Database.DateTimeTransform) : val;
}

/// <summary>
/// Gets the named column value.
Expand Down
16 changes: 14 additions & 2 deletions src/CoreEx/Abstractions/ETagGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,22 @@ public static class ETagGenerator
}

/// <summary>
/// Parses an <see cref="IETag.ETag"/> by removing double quotes character bookends; for example '<c>"abc"</c>' would be formatted as '<c>abc</c>'.
/// Parses an <see cref="IETag.ETag"/> by removing any weak prefix ('<c>W/</c>') double quotes character bookends; for example '<c>"abc"</c>' would be formatted as '<c>abc</c>'.
/// </summary>
/// <param name="etag">The <see cref="IETag.ETag"/> to unformat.</param>
/// <returns>The unformatted value.</returns>
public static string? ParseETag(string? etag) => etag is not null && etag.Length > 1 && etag.StartsWith("\"", StringComparison.InvariantCultureIgnoreCase) && etag.EndsWith("\"", StringComparison.InvariantCultureIgnoreCase) ? etag[1..^1] : etag;
public static string? ParseETag(string? etag)
{
if (string.IsNullOrEmpty(etag))
return null;

if (etag.StartsWith('\"') && etag.EndsWith('\"'))
return etag[1..^1];

if (etag.StartsWith("W/\"") && etag.EndsWith('\"'))
return etag[2..^1];

return etag;
}
}
}
13 changes: 3 additions & 10 deletions src/CoreEx/ExecutionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public static object GetRequiredService(Type type)
/// <summary>
/// Gets the <see cref="ServiceProvider"/>.
/// </summary>
/// <remarks>This is automatically set via the <see cref="Microsoft.Extensions.DependencyInjection.IServiceCollectionExtensions.AddExecutionContext(IServiceCollection, Func{IServiceProvider, ExecutionContext}?)"/>.</remarks>
/// <remarks>This is automatically set via the <see cref="IServiceCollectionExtensions.AddExecutionContext(IServiceCollection, Func{IServiceProvider, ExecutionContext}?)"/>.</remarks>
public IServiceProvider? ServiceProvider { get; set; }

/// <summary>
Expand All @@ -151,11 +151,6 @@ public static object GetRequiredService(Type type)
/// </summary>
public bool IsTextSerializationEnabled { get; set; }

/// <summary>
/// Gets or sets the <b>result</b> entity tag (used where the value does not explicitly implement <see cref="IETag"/>).
/// </summary>
public string? ResultETag { get; set; }

/// <summary>
/// Gets or sets the corresponding user name.
/// </summary>
Expand Down Expand Up @@ -202,15 +197,14 @@ public virtual ExecutionContext CreateCopy()
{
var ec = Create == null ? throw new InvalidOperationException($"The {nameof(Create)} function must not be null to create a copy.") : Create();
ec._timestamp = _timestamp;
ec._referenceDataContext = _referenceDataContext;
ec._messages = _messages;
ec._properties = _properties;
ec._referenceDataContext = _referenceDataContext;
ec._roles = _roles;
ec.ServiceProvider = ServiceProvider;
ec.CorrelationId = CorrelationId;
ec.OperationType = OperationType;
ec.IsTextSerializationEnabled = IsTextSerializationEnabled;
ec.ResultETag = ResultETag;
ec.UserName = UserName;
ec.UserId = UserId;
ec.TenantId = TenantId;
Expand All @@ -229,7 +223,6 @@ public void Dispose()
if (!_disposed)
{
_disposed = true;
Reset();
Dispose(true);
}
}
Expand All @@ -243,7 +236,7 @@ public void Dispose()
/// Releases the unmanaged resources used by the <see cref="ExecutionContext"/> and optionally releases the managed resources.
/// </summary>
/// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
protected virtual void Dispose(bool disposing) { }
protected virtual void Dispose(bool disposing) => Reset();

#region Security

Expand Down
2 changes: 1 addition & 1 deletion src/CoreEx/Http/HttpExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public static HttpRequestMessage ApplyRequestOptions(this HttpRequestMessage htt
/// <param name="httpRequest">The <see cref="HttpRequestMessage"/>.</param>
/// <param name="etag">The <i>ETag</i> value.</param>
/// <returns>The <see cref="HttpRequestMessage"/> to support fluent-style method-chaining.</returns>
/// <remarks>Automatically adds quoting to be ETag format compliant.</remarks>
/// <remarks>Automatically adds quoting to be ETag format compliant and sets the ETag as weak ('<c>W/</c>').</remarks>
public static HttpRequestMessage ApplyETag(this HttpRequestMessage httpRequest, string? etag)
{
// Apply the ETag header.
Expand Down
1 change: 0 additions & 1 deletion src/CoreEx/Http/TypedHttpClientBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ private async Task<HttpRequestMessage> CreateRequestInternalAsync(HttpMethod met

// Access the query string.
var uri = new Uri(requestUri, UriKind.RelativeOrAbsolute);

var ub = new UriBuilder(uri.IsAbsoluteUri ? uri : new Uri(new Uri("https://coreex"), requestUri));
var qs = HttpUtility.ParseQueryString(ub.Query);

Expand Down
2 changes: 1 addition & 1 deletion src/CoreEx/Json/IJsonPreFilterInspector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace CoreEx.Json
public interface IJsonPreFilterInspector
{
/// <summary>
/// Gets the underlying JSON object (as per the underlying implementation).
/// Gets the underlying JSON object (as per the underlying <see cref="IJsonSerializer"/> implementation).
/// </summary>
object Json { get; }

Expand Down
28 changes: 25 additions & 3 deletions tests/CoreEx.Test/Framework/WebApis/WebApiTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,24 @@ public void GetAsync_WithETagValueNotModified()

[Test]
public void GetAsync_WithGenETagValue()
{
using var test = FunctionTester.Create<Startup>();
var vcr = test.Type<WebApi>()
.Run(f => f.GetAsync(test.CreateHttpRequest(HttpMethod.Get, "https://unittest/testget"), r => Task.FromResult(new Person { Id = 1, Name = "Angela" })))
.ToActionResultAssertor()
.AssertOK()
.Result as ValueContentResult;

Assert.That(vcr, Is.Not.Null);
Assert.That(vcr!.ETag, Is.EqualTo("iVsGVb/ELj5dvXpe3ImuOy/vxLIJnUtU2b8nIfpX5PM="));

var p = test.JsonSerializer.Deserialize<Person>(vcr.Content!);
Assert.That(p, Is.Not.Null);
Assert.That(p.ETag, Is.EqualTo("iVsGVb/ELj5dvXpe3ImuOy/vxLIJnUtU2b8nIfpX5PM="));
}

[Test]
public void GetAsync_WithGenETagValue_QueryString()
{
using var test = FunctionTester.Create<Startup>();
var vcr = test.Type<WebApi>()
Expand All @@ -229,15 +247,19 @@ public void GetAsync_WithGenETagValue()
.Result as ValueContentResult;

Assert.That(vcr, Is.Not.Null);
Assert.That(vcr!.ETag, Is.EqualTo("F/BIL6G5jbvZxc4fnCc5BekkmFM9/BuXSSl/i5bQj5Q="));
Assert.That(vcr!.ETag, Is.EqualTo("cpDn3xugV1xKSHF9AY4kQRNQ1yC/SU49xC66C92WZbE="));

var p = test.JsonSerializer.Deserialize<Person>(vcr.Content!);
Assert.That(p, Is.Not.Null);
Assert.That(p.ETag, Is.EqualTo("iVsGVb/ELj5dvXpe3ImuOy/vxLIJnUtU2b8nIfpX5PM="));
}

[Test]
public void GetAsync_WithGenETagValueNotModified()
{
using var test = FunctionTester.Create<Startup>();
var hr = test.CreateHttpRequest(HttpMethod.Get, "https://unittest/testget?fruit=apples");
hr.Headers.Add(HeaderNames.IfMatch, "\\W\"F/BIL6G5jbvZxc4fnCc5BekkmFM9/BuXSSl/i5bQj5Q=\"");
hr.Headers.Add(HeaderNames.IfMatch, "\\W\"cpDn3xugV1xKSHF9AY4kQRNQ1yC/SU49xC66C92WZbE=\"");

test.Type<WebApi>()
.Run(f => f.GetAsync(hr, r => Task.FromResult(new Person { Id = 1, Name = "Angela" })))
Expand Down Expand Up @@ -650,7 +672,7 @@ public void PatchAsync_AutoConcurrency_Matched()
.Run(f => f.PatchAsync(hr, get: _ => Task.FromResult<Person?>(new Person { Id = 13, Name = "Deano" }), put: _ => Task.FromResult<Person>(new Person { Id = 13, Name = "Gazza" }), simulatedConcurrency: true))
.ToActionResultAssertor()
.AssertOK()
.AssertValue(new Person { Id = 13, Name = "Gazza" });
.AssertValue(new Person { Id = 13, Name = "Gazza", ETag = "tEEokPXk+4Q5MoiGqyAs1+6A00e2ww59Zm57LJgvBcg=" });
}

private static HttpRequest CreatePatchRequest(UnitTestEx.NUnit.Internal.FunctionTester<Startup> test, string? json, string? etag = null)
Expand Down
Loading

0 comments on commit 448792c

Please sign in to comment.