fix(workflows): preserve last-good variable on storage serialize failure (#8139)
Closes #8129. WorkflowInstanceStorageDriver: serialize failure keeps last-good value (no destructive Remove); read convert failure returns null / StrictMode throws instead of poison untyped JsonNode.
This commit is contained in:
parent
1ca11c23c8
commit
d0c7653ccf
|
|
@ -37,10 +37,8 @@ public class WorkflowInstanceStorageDriver(IPayloadSerializer payloadSerializer,
|
|||
}
|
||||
catch (Exception ex) when (ex is JsonException or NotSupportedException or ObjectDisposedException)
|
||||
{
|
||||
logger.LogWarning(ex, "Failed to serialize variable '{VariableId}' of type '{VariableType}' for workflow instance storage. The variable will be skipped.",
|
||||
logger.LogWarning(ex, "Failed to serialize variable '{VariableId}' of type '{VariableType}' for workflow instance storage. The stored value was left unchanged.",
|
||||
id, value?.GetType().FullName ?? "null");
|
||||
|
||||
dictionary.Remove(id);
|
||||
}
|
||||
});
|
||||
return ValueTask.CompletedTask;
|
||||
|
|
@ -59,8 +57,16 @@ public class WorkflowInstanceStorageDriver(IPayloadSerializer payloadSerializer,
|
|||
SerializerOptions = payloadSerializer.GetOptions()
|
||||
};
|
||||
var result = node.TryConvertTo(variableType, options);
|
||||
var parsedValue = result.IsSuccess ? result.Value : node;
|
||||
return new (parsedValue);
|
||||
if (result.IsSuccess)
|
||||
return new(result.Value);
|
||||
|
||||
logger.LogWarning(result.Exception, "Failed to convert stored variable '{VariableId}' to type '{VariableType}'. The stored value was left unread rather than returned as an untyped node.",
|
||||
id, variableType.FullName);
|
||||
|
||||
if (ObjectConverter.StrictMode)
|
||||
result.ThrowIfFailure();
|
||||
|
||||
return new((object?)null);
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
|
|
|
|||
|
|
@ -0,0 +1,137 @@
|
|||
using System.Text.Json;
|
||||
using System.Text.Json.Nodes;
|
||||
using Elsa.Expressions.Helpers;
|
||||
using Elsa.Workflows.Memory;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using NSubstitute;
|
||||
|
||||
namespace Elsa.Workflows.Core.UnitTests.VariableStorageDrivers;
|
||||
|
||||
[Collection(nameof(WorkflowInstanceStorageDriverTestsCollection))]
|
||||
public class WorkflowInstanceStorageDriverTests
|
||||
{
|
||||
[Fact]
|
||||
public async Task WriteAsync_WhenSerializeFails_PreservesPreviousValue()
|
||||
{
|
||||
var harness = CreateHarness(new Variable<string>("name", "kept"));
|
||||
const string id = "nameVariable";
|
||||
|
||||
await harness.Driver.WriteAsync(id, "kept", harness.Context);
|
||||
|
||||
var storedBefore = GetVariables(harness.Properties)[id].ToJsonString();
|
||||
|
||||
await harness.Driver.WriteAsync(id, CreateUnserializableValue(), harness.Context);
|
||||
|
||||
var dictionary = GetVariables(harness.Properties);
|
||||
Assert.True(dictionary.ContainsKey(id));
|
||||
Assert.Equal(storedBefore, dictionary[id].ToJsonString());
|
||||
|
||||
var read = await harness.Driver.ReadAsync(id, harness.Context);
|
||||
Assert.Equal("kept", read);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task WriteAsync_WhenSerializeFailsWithNoPriorValue_DoesNotCreateEntry()
|
||||
{
|
||||
var harness = CreateHarness(new Variable<string>("name", ""));
|
||||
const string id = "nameVariable";
|
||||
|
||||
await harness.Driver.WriteAsync(id, CreateUnserializableValue(), harness.Context);
|
||||
|
||||
Assert.False(GetVariables(harness.Properties).ContainsKey(id));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DeleteAsync_RemovesStoredValue()
|
||||
{
|
||||
var harness = CreateHarness(new Variable<string>("name", "kept"));
|
||||
const string id = "nameVariable";
|
||||
|
||||
await harness.Driver.WriteAsync(id, "kept", harness.Context);
|
||||
await harness.Driver.DeleteAsync(id, harness.Context);
|
||||
|
||||
Assert.False(GetVariables(harness.Properties).ContainsKey(id));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ReadAsync_WhenConvertFails_DoesNotReturnUntypedJsonNode()
|
||||
{
|
||||
var harness = CreateHarness(new Variable<int>("count", 0));
|
||||
const string id = "countVariable";
|
||||
SeedIncompatibleNode(harness.Properties, id);
|
||||
|
||||
var read = await harness.Driver.ReadAsync(id, harness.Context);
|
||||
|
||||
Assert.Null(read);
|
||||
Assert.False(read is JsonNode);
|
||||
Assert.True(GetVariables(harness.Properties).ContainsKey(id));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ReadAsync_WhenConvertFailsAndStrictMode_Throws()
|
||||
{
|
||||
var harness = CreateHarness(new Variable<int>("count", 0));
|
||||
const string id = "countVariable";
|
||||
SeedIncompatibleNode(harness.Properties, id);
|
||||
|
||||
var originalStrictMode = ObjectConverter.StrictMode;
|
||||
try
|
||||
{
|
||||
ObjectConverter.StrictMode = true;
|
||||
|
||||
await Assert.ThrowsAnyAsync<Exception>(() => harness.Driver.ReadAsync(id, harness.Context).AsTask());
|
||||
|
||||
Assert.True(GetVariables(harness.Properties).ContainsKey(id));
|
||||
}
|
||||
finally
|
||||
{
|
||||
ObjectConverter.StrictMode = originalStrictMode;
|
||||
}
|
||||
}
|
||||
|
||||
private static Harness CreateHarness(Variable variable)
|
||||
{
|
||||
var properties = new Dictionary<string, object>();
|
||||
var executionContext = Substitute.For<IExecutionContext>();
|
||||
executionContext.Properties.Returns(properties);
|
||||
|
||||
var payloadSerializer = Substitute.For<IPayloadSerializer>();
|
||||
payloadSerializer.GetOptions().Returns(new JsonSerializerOptions());
|
||||
|
||||
var driver = new WorkflowInstanceStorageDriver(payloadSerializer, NullLogger<WorkflowInstanceStorageDriver>.Instance);
|
||||
var context = new StorageDriverContext(executionContext, variable, CancellationToken.None);
|
||||
|
||||
return new(driver, context, properties);
|
||||
}
|
||||
|
||||
private static VariablesDictionary GetVariables(IDictionary<string, object> properties) =>
|
||||
(VariablesDictionary)properties[WorkflowInstanceStorageDriver.VariablesDictionaryStateKey];
|
||||
|
||||
private static void SeedIncompatibleNode(IDictionary<string, object> properties, string id)
|
||||
{
|
||||
properties[WorkflowInstanceStorageDriver.VariablesDictionaryStateKey] = new VariablesDictionary
|
||||
{
|
||||
[id] = JsonNode.Parse("""{"foo":"bar"}""")!
|
||||
};
|
||||
}
|
||||
|
||||
private static CyclicValue CreateUnserializableValue()
|
||||
{
|
||||
var value = new CyclicValue();
|
||||
value.Self = value;
|
||||
return value;
|
||||
}
|
||||
|
||||
private sealed record Harness(
|
||||
WorkflowInstanceStorageDriver Driver,
|
||||
StorageDriverContext Context,
|
||||
IDictionary<string, object> Properties);
|
||||
|
||||
private sealed class CyclicValue
|
||||
{
|
||||
public CyclicValue Self { get; set; } = null!;
|
||||
}
|
||||
}
|
||||
|
||||
[CollectionDefinition(nameof(WorkflowInstanceStorageDriverTestsCollection), DisableParallelization = true)]
|
||||
public sealed class WorkflowInstanceStorageDriverTestsCollection;
|
||||
Loading…
Reference in a new issue