Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 40 additions & 1 deletion SW.Bitween.Api/Resources/Documents/Update.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore;
using SW.Bitween.Domain.Accounts;
using System.Text.RegularExpressions;

namespace SW.Bitween.Resources.Documents
{
public class Update : ICommandHandler<int, DocumentUpdate,object>
public class Update : ICommandHandler<int, DocumentUpdate, object>
{
private readonly BitweenDbContext _dbContext;
private readonly IInfolinkCache _BitweenCache;
Expand Down Expand Up @@ -43,6 +44,44 @@ public async Task<object> Handle(int key, DocumentUpdate model)
throw new SWValidationException("DUPLICATED_BUS_TYPE_NAME",
"Cant use duplicated bus Message type name");

if (model.PromotedProperties != null)
{
foreach (var pp in model.PromotedProperties)
{
if (string.IsNullOrWhiteSpace(pp.Key))
throw new SWValidationException("INVALID_PROMOTED_PROPERTY_KEY",
"Promoted property key cannot be null or empty.");

if (string.IsNullOrWhiteSpace(pp.Value))
throw new SWValidationException("INVALID_PROMOTED_PROPERTY_VALUE",
$"Promoted property '{pp.Key}' must have a non-empty path value.");

if (model.DocumentFormat == DocumentFormat.Json)
{
// Must be a JSONPath: starts with '$' or a simple dot-separated identifier path
var trimmed = pp.Value.Trim();
if (!trimmed.StartsWith("$") && !Regex.IsMatch(trimmed, @"^[a-zA-Z_][a-zA-Z0-9_]*(?:(\.[a-zA-Z_][a-zA-Z0-9_]*)|(\[[0-9]+\]))*$"))
throw new SWValidationException("INVALID_PROMOTED_PROPERTY_PATH",
$"Promoted property '{pp.Key}' has an invalid JSON path: '{pp.Value}'. Expected a JSONPath expression (e.g. '$.field.subField') or dot-notation path.");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
else if (model.DocumentFormat == DocumentFormat.Xml)
{
// Basic XPath sanity: must start with '/' or '//' or be a valid element path
var trimmed = pp.Value.Trim();
if (!trimmed.StartsWith("/") && !Regex.IsMatch(trimmed, @"^[a-zA-Z_][a-zA-Z0-9_/\[\]@.:*-]*$"))
throw new SWValidationException("INVALID_PROMOTED_PROPERTY_PATH",
$"Promoted property '{pp.Key}' has an invalid XML path: '{pp.Value}'. Expected an XPath expression (e.g. '/root/element').");
}
}

var duplicateKey = model.PromotedProperties
.GroupBy(pp => pp.Key, System.StringComparer.OrdinalIgnoreCase)
.FirstOrDefault(g => g.Count() > 1)?.Key;

if (duplicateKey != null)
throw new SWValidationException("DUPLICATE_PROMOTED_PROPERTY_KEY",
$"Promoted property key '{duplicateKey}' appears more than once.");
}

var trail = new DocumentTrail(DocumentTrailCode.Updated, entity);
entity.SetDictionaries(model.PromotedProperties.ToDictionary());
Expand Down
25 changes: 24 additions & 1 deletion SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Scriban;
using Scriban.Parsing;
using Scriban.Runtime;

namespace SW.Bitween.NativeAdapters.JsonMapper;
Expand Down Expand Up @@ -102,11 +103,33 @@ private static ScriptObject BuildScriptObject(JObject obj)
private static object? ToScribanValue(JToken token) => token switch
{
JObject o => BuildScriptObject(o),
JArray a => a.Select(ToScribanValue).ToList(),
JArray a => new SmartArray(a.Select(ToScribanValue)),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Risk of silent data loss when accessing multi-element arrays.

Converting all JArray instances to SmartArray enables data.field syntax for array elements, but when an array contains multiple elements, only the first element is accessed. Template authors might not realize they're working with a multi-element array, leading to incomplete data processing.

Consider adding validation or logging when SmartArray member access is used on arrays with more than one element, or document this limitation prominently in template guidelines.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs` at line 106, The
conversion of JArray to SmartArray in the ToScribanValue mapping (JArray a =>
new SmartArray(a.Select(ToScribanValue))) can silently drop data when templates
use member access and only the first element is returned; update the JArray
handling in ToScribanValue/ToScribanValueEnumerable to detect multi-element
arrays and either (a) wrap them in a collection type that preserves all elements
for iteration, (b) log/warn via the existing logging facility when a SmartArray
is created from an array with Count > 1, or (c) throw a clear exception to force
template authors to handle multi-element arrays explicitly; reference JArray,
SmartArray, and ToScribanValue when making the change so reviewers can find the
mapping and then add the chosen validation/logging behavior and update any
docs/comments accordingly.

JValue v => v.Value,
_ => null
};

/// <summary>
/// A Scriban array that also delegates member access to its first element,
/// so templates can write either <c>data[0].field</c> or <c>data.field</c>
/// when the source JSON value is a single-element (or first-item) array.
/// </summary>
private sealed class SmartArray : ScriptArray
{
public SmartArray(IEnumerable<object?> items) : base(items) { }

public override bool TryGetValue(TemplateContext context, SourceSpan span, string member, out object? value)
{
if (base.TryGetValue(context, span, member, out value))
return true;

if (Count > 0 && this[0] is ScriptObject first)
return first.TryGetValue(context, span, member, out value);

value = null;
return false;
}
}

/// <summary>Sets a value at a dot-separated path inside a JObject, creating intermediate objects as needed.</summary>
private static void SetByPath(JObject root, string path, JToken value)
{
Expand Down