feat: add PartnerId to Xchange and inject partner/globals into Scriban templates - #149
Merged
Merged
Conversation
…n templates - Add PartnerId property to Xchange domain entity, propagated from subscription or gateway partner through all constructors including retries - Add AddPartnerIdToXchange migration for MsSql, MySql, and PgSql - Inject __partner__ adapter properties and __globals__ (all GlobalAdapterValuesSets) into xchange input JSON before Scriban template rendering in XchangeService - Update mapper preview endpoint (Preview.cs) to accept optional PartnerId, injecting the same __partner__ and __globals__ context for accurate live preview
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 4 minutes and 27 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request adds a Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant PreviewHandler as Preview Handler
participant DbContext as DbContext
participant Partner as Partner Entity
participant GlobalSets as GlobalAdapterValuesSet
participant Scriban as Scriban Template
Client->>PreviewHandler: MapperPreviewRequest (PartnerId)
PreviewHandler->>PreviewHandler: Parse InputJson to JObject
alt PartnerId provided
PreviewHandler->>DbContext: Load Partner by PartnerId
DbContext-->>PreviewHandler: Partner with AdapterProperties
PreviewHandler->>PreviewHandler: Inject Partner data as __partner__
end
PreviewHandler->>DbContext: Load all GlobalAdapterValuesSets
DbContext-->>PreviewHandler: GlobalAdapterValuesSet entities
PreviewHandler->>PreviewHandler: Inject non-empty Values as __globals__
alt Enrichment occurred
PreviewHandler->>PreviewHandler: Serialize enriched JSON
end
PreviewHandler->>Scriban: Render template with enriched input
Scriban-->>PreviewHandler: Rendered output
PreviewHandler-->>Client: MapperPreviewResponse
sequenceDiagram
participant XchangeService
participant XchangeFile as XchangeFile Data
participant DbContext as DbContext
participant Partner as Partner Entity
participant GlobalSets as GlobalAdapterValuesSet
participant Adapter as Native/Serverless Adapter
XchangeService->>XchangeFile: Parse xchangeFile.Data to JObject
alt xchange.MapperId exists
alt xchange.PartnerId provided
XchangeService->>DbContext: Load Partner by PartnerId
DbContext-->>XchangeService: Partner with AdapterProperties
XchangeService->>XchangeService: Inject Partner data as __partner__
end
XchangeService->>DbContext: Load all GlobalAdapterValuesSets
DbContext-->>XchangeService: GlobalAdapterValuesSet entities
XchangeService->>XchangeService: Inject Values as __globals__
alt Enrichment occurred
XchangeService->>XchangeFile: Update with enriched JSON
end
end
XchangeService->>Adapter: Pass enriched xchangeFile
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SW.Bitween.Api/Services/XchangeService.cs (1)
132-165:⚠️ Potential issue | 🔴 CriticalCritical: unconditional
JObject.Parsewill break every non-JSON mapper, and enrichment is applied to mappers that don’t use Scriban.This block runs for every
RunMapperinvocation — native or serverless adapter, JSON or not. Concrete problems:
- Non-JSON inputs crash.
JObject.Parse(xchangeFile.Data)on an XML/CSV/fixed-width/binary payload throwsJsonReaderException, regressing any mapper whose input isn’t JSON. The previous code path handed the payload through untouched. Gate the parse on a known content type (e.g.,xchangeFile.ContentType/InputContentTypestarts withapplication/json) or wrap in a targetedtry/catchthat skips enrichment on non-JSON.- Scope too broad. The comment says “so Scriban templates can reference them,” but the enrichment happens for every adapter. Native adapters that hash/validate input, serverless adapters expecting a clean domain payload, etc. will all receive mutated JSON with injected
__partner__/__globals__. Consider restricting enrichment to template/Scriban mappers (e.g., detect byMapperIdprefix or an explicit capability flag on the adapter).- Silent key collision. If the incoming JSON already contains
__partner__or__globals__, the injection overwrites them with no warning. At minimum, choose names unlikely to collide and/or log when an existing key is clobbered.- Hot-path DB query.
_dbContext.Set<GlobalAdapterValuesSet>().ToListAsync()is executed on every exchange. Globals change rarely — please route this through_BitweenCache(consistent with howSubscription/Document/WorkGroup/Notifierare already cached) to avoid N queries-per-request pressure on the DB under load.- Consistency with
Preview.cs. The preview path does the same enrichment per PR description — any fix here should mirror there so live preview matches production rendering.🛠 Sketch of a safer enrichment path
- // Inject __partner__ adapter properties into the input JSON so Scriban templates - // can reference them as {{ __partner__?.propkey }} - var jObjEnriched = JObject.Parse(xchangeFile.Data); - var enriched = false; - - if (xchange.PartnerId.HasValue) - { - var partner = await _dbContext.FindAsync<Partner>(xchange.PartnerId.Value); - if (partner?.AdapterProperties?.Count > 0) - { - jObjEnriched["__partner__"] = JObject.FromObject(partner.AdapterProperties); - enriched = true; - } - } - - // Inject __globals__ — all global adapter values sets - // so templates can use {{ __globals__?.setId?.key }} - var globalSets = await _dbContext.Set<GlobalAdapterValuesSet>().ToListAsync(); - if (globalSets.Any(s => s.Values?.Count > 0)) - { - var globalsObj = new JObject(); - foreach (var set in globalSets.Where(s => s.Values?.Count > 0)) - globalsObj[set.Id] = JObject.FromObject(set.Values); - jObjEnriched["__globals__"] = globalsObj; - enriched = true; - } - - if (enriched) - xchangeFile = new XchangeFile(jObjEnriched.ToString(Formatting.None), xchangeFile.Filename); + // Only template-style mappers consume __partner__/__globals__, and only JSON payloads + // can be enriched safely. Skip otherwise to preserve existing behavior for native/binary flows. + if (ShouldEnrichForTemplate(xchange) && LooksLikeJson(xchangeFile)) + { + if (TryParseJson(xchangeFile.Data, out var jObjEnriched)) + { + var enriched = false; + + if (xchange.PartnerId.HasValue) + { + var partner = await _dbContext.FindAsync<Partner>(xchange.PartnerId.Value); + if (partner?.AdapterProperties?.Count > 0) + { + if (jObjEnriched.ContainsKey("__partner__")) + _logger.LogWarning("Overwriting existing __partner__ key in mapper input for xchange {Id}", xchange.Id); + jObjEnriched["__partner__"] = JObject.FromObject(partner.AdapterProperties); + enriched = true; + } + } + + var globalSets = await _BitweenCache.ListGlobalAdapterValuesSetsAsync(); // add cached accessor + // ... same globals injection, guarded by ContainsKey check ... + + if (enriched) + xchangeFile = new XchangeFile(jObjEnriched.ToString(Formatting.None), xchangeFile.Filename); + } + }Operational note — missing FK on
Xchanges.PartnerId. Across the three new migrations (MsSql/MySql/PgSql),PartnerIdis added as a plain nullableintwith no foreign key toPartners(Id)and no index.Subscription.PartnerIdhas both an FK (PgSql/MsSql) and anix_subscription_partner_idindex. Consider adding the same forXchange.PartnerIdfor referential integrity and for efficient lookups if anything ever queries xchanges by partner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SW.Bitween.Api/Services/XchangeService.cs` around lines 132 - 165, RunMapper currently unconditionally parses xchangeFile.Data as JSON and injects __partner__/__globals__ for every mapper; change it to: only attempt JObject.Parse when the input content type is JSON (e.g., check xchangeFile.ContentType or InputContentType startsWith "application/json") and bail out on parse errors to preserve non-JSON inputs; restrict enrichment to Scriban/template mappers (detect via MapperId pattern or an explicit mapper capability flag) so native/serverless adapters aren’t mutated; before writing __partner__ or __globals__ check for existing keys and log a warning if you would overwrite them; replace the per-request _dbContext.Set<GlobalAdapterValuesSet>().ToListAsync() call with a cached lookup via _BitweenCache (same cache strategy as Subscription/Document/WorkGroup/Notifier) and apply the identical changes to the preview path (Preview.cs) so behavior matches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@SW.Bitween.Api/Resources/Mappers/Preview.cs`:
- Around line 75-78: The current catch-all in the Preview mapping code (catch
(Exception ex) returning new MapperPreviewResponse { Error = ex.Message }) leaks
internal errors to clients; change it to either narrow the try/catch to only the
JSON/template rendering portion or catch specific exceptions (e.g.,
JsonException/FormatException) and for all other exceptions log the full
exception server-side via the existing logger (e.g., ILogger or process logger
used in this class) and return a generic MapperPreviewResponse with a
non-sensitive message like "Internal error generating preview" (keep
MapperPreviewResponse.Error generic), ensuring database/EF exceptions are not
returned verbatim.
- Around line 44-46: The change unconditionally calls JObject.Parse on InputJson
(variable inputJson) which throws for valid non-object JSON and alters previous
behavior of passing InputJson through to ScribanJsonHelper.Render; revert to
safely attempting to parse and only run enrichment and final serialization when
jObj != null: wrap the JObject.Parse attempt in a try/nullable pattern (set jObj
to null on non-object/parse failure), guard the enrichment blocks that reference
jObj and the serialization step with jObj != null, and when jObj is null
continue to call ScribanJsonHelper.Render(inputJson, ...) as before so
non-object roots keep the original behavior; refer to InputJson, jObj, enriched,
JObject.Parse and ScribanJsonHelper.Render to locate the affected logic.
- Around line 59-67: The mapper currently materializes all
GlobalAdapterValuesSet via
_dbContext.Set<GlobalAdapterValuesSet>().ToListAsync() then checks .Any(...) and
iterates .Where(...) causing double enumeration and attempting a server-side
predicate on the JSON-valued Values property; change this to a single
client-side pass and use the app's caching layer for GlobalAdapterValuesSet
instead of querying the DB directly: remove the initial Any(...) check, obtain
the cached list (replace the ToListAsync call with the cache accessor for
GlobalAdapterValuesSet), then iterate once over that list checking Values?.Count
> 0 and building globalsObj (refer to the globalSets variable and Values
property in Preview.cs) so you only enumerate once and avoid server-side JSON
predicates.
In `@SW.Bitween.MySql/Migrations/BitweenDbContextModelSnapshot.cs`:
- Line 20: The PgSql ModelSnapshot's metadata ProductVersion is out of sync with
the MsSql/MySql snapshots; regenerate the PostgreSQL context snapshot using the
same dotnet-ef/EF Core tools version used for the others so the ProductVersion
annotation matches (e.g., update the ProductVersion string from "8.0.23" to
"8.0.26" by re-running the snapshot generation for the PostgreSQL DbContext).
Locate the ProductVersion annotation in the PgSql ModelSnapshot (the
.HasAnnotation("ProductVersion", ...) entry in the snapshot class) and either
regenerate the snapshot with the matching tooling or manually align the
annotation to the same version string used by BitweenDbContextModelSnapshot.cs
to prevent spurious diffs.
---
Outside diff comments:
In `@SW.Bitween.Api/Services/XchangeService.cs`:
- Around line 132-165: RunMapper currently unconditionally parses
xchangeFile.Data as JSON and injects __partner__/__globals__ for every mapper;
change it to: only attempt JObject.Parse when the input content type is JSON
(e.g., check xchangeFile.ContentType or InputContentType startsWith
"application/json") and bail out on parse errors to preserve non-JSON inputs;
restrict enrichment to Scriban/template mappers (detect via MapperId pattern or
an explicit mapper capability flag) so native/serverless adapters aren’t
mutated; before writing __partner__ or __globals__ check for existing keys and
log a warning if you would overwrite them; replace the per-request
_dbContext.Set<GlobalAdapterValuesSet>().ToListAsync() call with a cached lookup
via _BitweenCache (same cache strategy as
Subscription/Document/WorkGroup/Notifier) and apply the identical changes to the
preview path (Preview.cs) so behavior matches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b436428f-932c-48f9-ac88-229c9667b1df
📒 Files selected for processing (12)
SW.Bitween.Api/Domain/Xchange/Xchange.csSW.Bitween.Api/Resources/Mappers/Preview.csSW.Bitween.Api/Services/XchangeService.csSW.Bitween.MsSql/Migrations/20260419090137_AddPartnerIdToXchange.Designer.csSW.Bitween.MsSql/Migrations/20260419090137_AddPartnerIdToXchange.csSW.Bitween.MsSql/Migrations/BitweenDbContextModelSnapshot.csSW.Bitween.MySql/Migrations/20260419090224_AddPartnerIdToXchange.Designer.csSW.Bitween.MySql/Migrations/20260419090224_AddPartnerIdToXchange.csSW.Bitween.MySql/Migrations/BitweenDbContextModelSnapshot.csSW.Bitween.PgSql/Migrations/20260419092109_AddPartnerIdToXchange.Designer.csSW.Bitween.PgSql/Migrations/20260419092109_AddPartnerIdToXchange.csSW.Bitween.PgSql/Migrations/BitweenDbContextModelSnapshot.cs
Comment on lines
+59
to
+67
| var globalSets = await _dbContext.Set<GlobalAdapterValuesSet>().ToListAsync(); | ||
| if (globalSets.Any(s => s.Values?.Count > 0)) | ||
| { | ||
| var globalsObj = new JObject(); | ||
| foreach (var set in globalSets.Where(s => s.Values?.Count > 0)) | ||
| globalsObj[set.Id] = JObject.FromObject(set.Values); | ||
| jObj["__globals__"] = globalsObj; | ||
| enriched = true; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how GlobalAdapterValuesSet.Values is configured/mapped
fd -e cs GlobalAdapterValuesSet
rg -nP -C3 '\bGlobalAdapterValuesSet\b' --type=csRepository: simplify9/Bitween-api
Length of output: 24399
🏁 Script executed:
# Check EF Core configuration for GlobalAdapterValuesSet.Values property mapping
cat -n SW.Bitween.Api/Data/BitweenDbContext.cs | sed -n '115,130p'
cat -n SW.Bitween.PgSql/BitweenDbContext.cs | sed -n '140,160p'
rg -A15 'Entity<GlobalAdapterValuesSet>' SW.Bitween.Api/Data/BitweenDbContext.csRepository: simplify9/Bitween-api
Length of output: 2851
🏁 Script executed:
# Check Preview.cs full context and dependencies
cat -n SW.Bitween.Api/Resources/Mappers/Preview.cs | head -100
# Check if there's caching available
rg -B10 'class Preview' SW.Bitween.Api/Resources/Mappers/Preview.cs | head -20Repository: simplify9/Bitween-api
Length of output: 3523
Minor: double enumeration; filter predicate cannot translate to SQL.
globalSetsis filtered twice (.Any()on line 60, then.Where()on line 63), both on the in-memory list. SinceToListAsync()materializes all records, both operations enumerate the same collection.- The
Valuesproperty is stored as a JSON column (.StoreAsJson()in SQL Server,jsonbin PostgreSQL), so theValues?.Countpredicate cannot be evaluated server-side. The filter must remain client-side, making the proposed refactor ineffective.
Suggested fix:
- var globalSets = await _dbContext.Set<GlobalAdapterValuesSet>().ToListAsync();
- if (globalSets.Any(s => s.Values?.Count > 0))
- {
- var globalsObj = new JObject();
- foreach (var set in globalSets.Where(s => s.Values?.Count > 0))
- globalsObj[set.Id] = JObject.FromObject(set.Values);
- jObj["__globals__"] = globalsObj;
- enriched = true;
- }
+ var globalSets = await _dbContext.Set<GlobalAdapterValuesSet>().ToListAsync();
+ var nonEmptySets = globalSets.Where(s => s.Values?.Count > 0).ToList();
+ if (nonEmptySets.Count > 0)
+ {
+ var globalsObj = new JObject();
+ foreach (var set in nonEmptySets)
+ globalsObj[set.Id] = JObject.FromObject(set.Values);
+ jObj["__globals__"] = globalsObj;
+ enriched = true;
+ }Every preview request loads the full table; the application has a caching layer for GlobalAdapterValuesSet, but this mapper queries the database directly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var globalSets = await _dbContext.Set<GlobalAdapterValuesSet>().ToListAsync(); | |
| if (globalSets.Any(s => s.Values?.Count > 0)) | |
| { | |
| var globalsObj = new JObject(); | |
| foreach (var set in globalSets.Where(s => s.Values?.Count > 0)) | |
| globalsObj[set.Id] = JObject.FromObject(set.Values); | |
| jObj["__globals__"] = globalsObj; | |
| enriched = true; | |
| } | |
| var globalSets = await _dbContext.Set<GlobalAdapterValuesSet>().ToListAsync(); | |
| var nonEmptySets = globalSets.Where(s => s.Values?.Count > 0).ToList(); | |
| if (nonEmptySets.Count > 0) | |
| { | |
| var globalsObj = new JObject(); | |
| foreach (var set in nonEmptySets) | |
| globalsObj[set.Id] = JObject.FromObject(set.Values); | |
| jObj["__globals__"] = globalsObj; | |
| enriched = true; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.Api/Resources/Mappers/Preview.cs` around lines 59 - 67, The mapper
currently materializes all GlobalAdapterValuesSet via
_dbContext.Set<GlobalAdapterValuesSet>().ToListAsync() then checks .Any(...) and
iterates .Where(...) causing double enumeration and attempting a server-side
predicate on the JSON-valued Values property; change this to a single
client-side pass and use the app's caching layer for GlobalAdapterValuesSet
instead of querying the DB directly: remove the initial Any(...) check, obtain
the cached list (replace the ToListAsync call with the cache accessor for
GlobalAdapterValuesSet), then iterate once over that list checking Values?.Count
> 0 and building globalsObj (refer to the globalSets variable and Values
property in Preview.cs) so you only enumerate once and avoid server-side JSON
predicates.
…ues more efficiently
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
AhmadRAbuhussein
approved these changes
Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features