APIV2: replace LoadFields with AddLoadFields#29955
Conversation
Replace per-record Rec.LoadFields calls inside SetCalculatedFields and LoadCurrencyInformation procedures with Rec.AddLoadFields in OnOpenPage. LoadFields invoked from OnAfterGetRecord causes JIT reads on every record fetch, which triggers telemetry errors under concurrent load. AddLoadFields declared in OnOpenPage ensures the required fields are included in the initial dataset query, eliminating unnecessary JIT round-trips entirely. Affected pages: - APIV2SalesInvoices: fields No., Currency Code, Amount Including VAT, Posted, Status - APIV2SalesCreditMemos: fields Applies-to Doc. Type, Currency Code - APIV2SalesQuotes: field Currency Code - APIV2PurchaseInvoices: field Currency Code - APIV2PurchaseCreditMemos: fields Applies-to Doc. Type, Currency Code - APIV2BankAccounts: field Currency Code (new OnOpenPage trigger added) Fixes microsoft#29634
JesperSchulz
added
Integration
|
|
||
| trigger OnOpenPage() | ||
| begin | ||
| Rec.AddLoadFields("Currency Code"); |
There was a problem hiding this comment.
We had issues with this kind of approach in the past. API page does not behave 100% as the actual page. In some cases the platform would open and close the page. Many properties that would be set from the OnOpen page would be lost. I'm not sure if we still have this kind of behavior on e.g. delete, POST, get $batch and other operations that can be done.
There was a problem hiding this comment.
We would need an extensive testing before we uptake this change to address the risk.
There was a problem hiding this comment.
Good point. The BankAccounts page is different from the others in this PR because OnOpenPage was newly added just for AddLoadFields - there was no pre-existing OnOpenPage. This means that for API operations that may not trigger OnOpenPage (POST via OnInsertRecord, PATCH via OnModifyRecord, batch requests), AddLoadFields would not fire and LoadCurrencyInformation would fall back to a JIT read.
Switched OnOpenPage to OnInit for the BankAccounts page. OnInit fires when the page object is instantiated, before any trigger or data operation runs, making it the correct placement for AddLoadFields when there is no pre-existing OnOpenPage.
For the other pages in this PR (SalesInvoices, SalesCreditMemos, SalesQuotes, PurchaseInvoices, PurchaseCreditMemos), OnOpenPage already existed for permission checks. Adding AddLoadFields alongside those existing calls carries the same risk profile as the permission checks themselves, which are already relied on in production.
Regarding extensive testing - agreed. This change can be tested with API requests for GET (list and single), POST, PATCH, and DELETE on the Bank Accounts endpoint to confirm Currency Code is always correctly returned.
There was a problem hiding this comment.
We decided to skip this optimization for the time being to limit the risk.
OnOpenPage is not guaranteed to be called for all API page operations (POST, PATCH, DELETE, batch). OnInit is always called when the page object is instantiated, making it the correct trigger for AddLoadFields which must fire before any record data is fetched.
jeffreybulanadi
changed the title
|
Thank you for your contribution and for taking the time to submit this pull request. At this stage, we’re not able to accept the change. We’d like to revisit this proposal once the migration to BCApps is complete though. At that point, we’ll be in a better position to properly evaluate and validate changes like this. We appreciate your contribution and encourage you to resubmit or revisit this after the transition has been finalized. |
3 participants
Summary
APIV2 pages call Rec.LoadFields(...) inside SetCalculatedFields and LoadCurrencyInformation procedures, which are themselves called from OnAfterGetRecord. This causes a JIT (Just-In-Time) read from the database for every record fetched, resulting in telemetry errors under concurrent API load as reported in issue #29634.
Root Cause
Rec.LoadFields in OnAfterGetRecord forces a supplemental DB read per record. When many records are fetched concurrently the platform logs JIT load warnings and the performance degrades unnecessarily.
Fix
Replace Rec.LoadFields with Rec.AddLoadFields placed in OnOpenPage. AddLoadFields declares which fields must be included in the initial record-set query for the entire page session. The fields are fetched once alongside the primary key columns, removing the per-record JIT reads entirely.
Changes per page:
There is no behavioral change. The same fields are loaded; only the loading strategy changes from JIT per-record to eager per-session.
Fixes #29634
Fixes AB#632881