From d581bb706a81f13077691b854dbb3cb7f8c0a6f8 Mon Sep 17 00:00:00 2001 From: Morten Holt Date: Wed, 1 Jul 2026 15:45:57 +0200 Subject: [PATCH 1/2] Fix: calculated-field evaluation must not persist via a spurious Update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When XrmMockup evaluated a classic calculated field during a Retrieve/RetrieveMultiple, the parsed calc workflow reached its terminal SetAttributeValue node and issued a real orgService.Update of the record. That write was wrong regardless: a calculated field should only compute a value and project it onto the returned entity. The spurious Update fired the update pipeline, bumped modifiedon, and ran UpdateRequestHandler's HasCircularReference guard — which rejects any record legitimately carrying a self-referential lookup (e.g. a systemuser whose createdby is itself), surfacing as the "circular reference" FaultException in the bug report. Fix (smallest change satisfying the constraints): add a suppressWrites parameter to WorkflowTree.Execute. It sets a "SuppressWrites" sentinel into Variables *after* Reset() (which reinitializes Variables), and SetAttributeValue.Execute returns instead of calling orgService.Update when the sentinel is set. ExecuteCalculatedFields passes suppressWrites: true; the computed value is already left in the primaryEntity variable and copied back as before. All other callers (real workflows via WorkflowManager, rollups via CalculateRollupFieldRequestHandler) keep the default false and still persist. ExecuteFormulaFields uses the PowerFx evaluator and never reaches SetAttributeValue, so it needs no change. Removing the write exposed a pre-existing latent NRE in Utility.GetFormattedValueLabel: a Money attribute's formatted value dereferenced transactioncurrencyid without a null check. Previously the spurious Update ran HandleCurrencies and backfilled the currency; without the write, a calculated Money column can be projected onto a record that has no currency, and RetrieveMultiple's SetFormattedValues threw. Guard the Money branch to omit the formatted value when there is no currency, mirroring the tolerant Lookup branch beside it. Regression test (TestMoney.TestCalculatedFieldRetrieveDoesNotPersistAsUpdate): creates a ctx_parent, advances the mock clock, then reads the calculated Money column via both Retrieve and RetrieveMultiple. Before the fix modifiedon jumped forward by the clock advance (proving a spurious Update); after, it is unchanged and the calc value is still projected. Full net8.0 suite: 620 passed, 1 skipped, 0 failed (green across repeated runs). Co-Authored-By: Claude via Conducktor --- RELEASE_NOTES.md | 4 ++ src/XrmMockup365/Core.cs | 2 +- src/XrmMockup365/Internal/Utility.cs | 17 ++++++--- .../WorkflowNode/SetAttributeValue.cs | 10 +++++ src/XrmMockup365/Workflow/WorkflowTree.cs | 16 +++++++- tests/XrmMockup365Test/TestMoney.cs | 37 +++++++++++++++++++ 6 files changed, 79 insertions(+), 7 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a074ba50..bf46ddba 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,3 +1,7 @@ +### 1.18.5 - 1 July 2026 +* Fix: Evaluating a classic calculated field during Retrieve/RetrieveMultiple no longer issues a spurious `Update` of the record. The calculated value is now computed and projected onto the returned entity without persisting, so reads no longer bump `modifiedon`, fire update plugins, or trip the update-time circular-reference guard (which rejected records carrying a self-referential lookup, e.g. a `systemuser` whose `createdby` is itself). Real workflows and rollup fields still update as before. +* Fix: RetrieveMultiple no longer throws when building the formatted value for a Money attribute that has no associated `transactioncurrencyid` (e.g. a calculated Money column projected onto a record without a currency); the formatted value is simply omitted. + ### 1.18.4 - 1 July 2026 * Fix: AppendTo/security checks for business- and organization-owned entities (#339) diff --git a/src/XrmMockup365/Core.cs b/src/XrmMockup365/Core.cs index 010a835f..28503b27 100644 --- a/src/XrmMockup365/Core.cs +++ b/src/XrmMockup365/Core.cs @@ -1166,7 +1166,7 @@ internal void ExecuteCalculatedFields(EntityMetadata entityMetadata, Entity enti // the calculation workflow against a clone and copy the computed value back onto the // returned entity (the workflow leaves its result in the "primaryEntity" variable). var resultTree = tree.Execute(entity.CloneEntity(entityMetadata, new ColumnSet(true)), TimeOffset, GetWorkflowService(), - factory, factory.GetService()); + factory, factory.GetService(), suppressWrites: true); if (resultTree.Variables.TryGetValue("InputEntities(\"primaryEntity\")", out var resultObj) && resultObj is Entity result && result.Contains(attr.LogicalName)) diff --git a/src/XrmMockup365/Internal/Utility.cs b/src/XrmMockup365/Internal/Utility.cs index 85b5558b..dc44fa3b 100644 --- a/src/XrmMockup365/Internal/Utility.cs +++ b/src/XrmMockup365/Internal/Utility.cs @@ -883,12 +883,19 @@ private static string GetFormattedValueLabel(XrmDb db, AttributeMetadata metadat if (metadataAtt is MoneyAttributeMetadata) { - var currencysymbol = - db.GetEntity( - db.GetEntity(entity.ToEntityReference()) - .GetAttributeValue("transactioncurrencyid")) - .GetAttributeValue("currencysymbol"); + // A Money value can be present without an associated transactioncurrencyid — e.g. a + // calculated Money field projected onto the entity during a read (which, unlike a real + // write, does not run HandleCurrencies to backfill the currency). Without a currency we + // cannot resolve a symbol, so skip the formatted value rather than dereferencing a null + // currency reference (which previously threw and broke RetrieveMultiple's formatting). + var currencyRef = db.GetEntity(entity.ToEntityReference()) + .GetAttributeValue("transactioncurrencyid"); + if (currencyRef == null) + { + return null; + } + var currencysymbol = db.GetEntity(currencyRef).GetAttributeValue("currencysymbol"); return currencysymbol + (value as Money).Value.ToString(); } diff --git a/src/XrmMockup365/Workflow/WorkflowNode/SetAttributeValue.cs b/src/XrmMockup365/Workflow/WorkflowNode/SetAttributeValue.cs index f30bccc1..a42e9dc8 100644 --- a/src/XrmMockup365/Workflow/WorkflowNode/SetAttributeValue.cs +++ b/src/XrmMockup365/Workflow/WorkflowNode/SetAttributeValue.cs @@ -36,6 +36,16 @@ public void Execute(ref Dictionary variables, TimeSpan timeOffse throw new WorkflowException($"The variable with id '{VariableId}' before being set, check the workflow has the correct format."); } + // When evaluating a calculated/formula field during a read, persistence is suppressed: the + // computed value already lives in the "primaryEntity" variable for the caller to project onto + // the returned entity, so re-Updating the whole record would be a spurious write (firing the + // update pipeline, mutating data, and tripping the update-time circular-reference guard). + // Real workflows leave this flag unset and still persist. + if (variables.TryGetValue(WorkflowTree.SuppressWritesKey, out var suppress) && suppress is bool b && b) + { + return; + } + var entity = variables[VariableId] as Entity; orgService.Update(entity); diff --git a/src/XrmMockup365/Workflow/WorkflowTree.cs b/src/XrmMockup365/Workflow/WorkflowTree.cs index 8395cbef..5c6bdf34 100644 --- a/src/XrmMockup365/Workflow/WorkflowTree.cs +++ b/src/XrmMockup365/Workflow/WorkflowTree.cs @@ -101,14 +101,28 @@ public WorkflowTree(IWorkflowNode StartActivity, bool? TriggerOnCreate, bool? Tr this.Output = Output; } + // A calculated/formula field is evaluated on demand during a Retrieve and must never persist: + // it should only compute a value and leave it in the "primaryEntity" variable for the caller to + // project onto the returned entity. When suppressWrites is true the terminal SetAttributeValue + // node skips its orgService.Update, so calc evaluation cannot fire the update pipeline, mutate + // data, or trip the update-time circular-reference guard. Real workflows and rollups pass the + // default (false) so their genuine "update record" steps still write. + public const string SuppressWritesKey = "SuppressWrites"; + public WorkflowTree Execute(Entity primaryEntity, TimeSpan timeOffset, - IOrganizationService orgService, IOrganizationServiceFactory factory, ITracingService trace) + IOrganizationService orgService, IOrganizationServiceFactory factory, ITracingService trace, + bool suppressWrites = false) { if (primaryEntity.Id == Guid.Empty) { throw new WorkflowException("The primary entity must have an id"); } Reset(); + // Set after Reset(), which reinitializes Variables and would otherwise drop this flag. + if (suppressWrites) + { + Variables[SuppressWritesKey] = true; + } Variables["InputEntities(\"primaryEntity\")"] = primaryEntity; Variables["ExecutionTime"] = DateTime.Now.Add(timeOffset); var transactioncurrencyid = "transactioncurrencyid"; diff --git a/tests/XrmMockup365Test/TestMoney.cs b/tests/XrmMockup365Test/TestMoney.cs index d31fa6ae..8bc25d92 100644 --- a/tests/XrmMockup365Test/TestMoney.cs +++ b/tests/XrmMockup365Test/TestMoney.cs @@ -88,6 +88,43 @@ public void TestCalculatedIsSetRetrieveMultiple() // Dropped assertion on dg_AllConditions: no equivalent field exists on ctx_parent. } + // Regression for the calculated-field write-as-update bug: evaluating a calculated field during + // a Retrieve/RetrieveMultiple must only compute a value and project it onto the returned entity — + // it must never re-Update the record. Before the fix, calc evaluation reached the terminal + // SetAttributeValue workflow node which called orgService.Update; that spurious write ran the full + // update pipeline (bumping modifiedon, firing update plugins, and running UpdateRequestHandler's + // HasCircularReference guard — the "circular reference" FaultException in the original report). + // + // We detect the write via modifiedon: capture it after create, advance the mock clock, then read + // the calc field. If the spurious Update still fires, Touch() rewrites modifiedon to the advanced + // time; a compute-only read leaves it untouched. The clock advance makes the two cases + // unambiguously distinguishable. + [Fact] + public void TestCalculatedFieldRetrieveDoesNotPersistAsUpdate() + { + var bus = new ctx_parent { ctx_Name = "CalcRead", ctx_Amount = 30 }; + bus.Id = orgAdminService.Create(bus); + + var createdModifiedOn = orgAdminService + .Retrieve(ctx_parent.EntityLogicalName, bus.Id, new ColumnSet("modifiedon")) + .GetAttributeValue("modifiedon"); + + // Advance the clock so any spurious Touch() produces a clearly different modifiedon. + crm.AddDays(5); + + // Retrieve selecting the calculated column (ctx_AmountCalcClassic = ctx_Amount * 20). + var retrieved = ctx_parent.Retrieve(orgAdminService, bus.Id, x => x.ctx_AmountCalcClassic, x => x.ModifiedOn); + Assert.Equal(30m * 20, retrieved.ctx_AmountCalcClassic); // calc value still projected + // Before the fix modifiedon jumped forward 5 days (the spurious Update); after, it is unchanged. + Assert.Equal(createdModifiedOn, retrieved.ModifiedOn); + + // Same via RetrieveMultiple (the code path in the original bug report). + var q = new QueryExpression("ctx_parent") { ColumnSet = new ColumnSet(true) }; + var multi = (ctx_parent)orgAdminService.RetrieveMultiple(q).Entities.Single(e => e.Id == bus.Id); + Assert.Equal(30m * 20, multi.ctx_AmountCalcClassic); + Assert.Equal(createdModifiedOn, multi.ModifiedOn); + } + [Fact] public void TestRollUp() { From ca8043895a4c11b5b2ab14daa1a5c18d09c62568 Mon Sep 17 00:00:00 2001 From: Morten Holt Date: Wed, 1 Jul 2026 22:04:50 +0200 Subject: [PATCH 2/2] Set SuppressWrites flag unconditionally per execution Reset() does not clear the Variables dictionary, so setting SuppressWritesKey only when suppressWrites was true could leave a stale 'true' on a reused WorkflowTree instance, silently suppressing a later real workflow's Update. Assign the flag on every Execute so behavior depends only on the current call. Co-Authored-By: Claude via Conducktor --- src/XrmMockup365/Workflow/WorkflowTree.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/XrmMockup365/Workflow/WorkflowTree.cs b/src/XrmMockup365/Workflow/WorkflowTree.cs index 5c6bdf34..45f0775a 100644 --- a/src/XrmMockup365/Workflow/WorkflowTree.cs +++ b/src/XrmMockup365/Workflow/WorkflowTree.cs @@ -118,11 +118,11 @@ public WorkflowTree Execute(Entity primaryEntity, TimeSpan timeOffset, throw new WorkflowException("The primary entity must have an id"); } Reset(); - // Set after Reset(), which reinitializes Variables and would otherwise drop this flag. - if (suppressWrites) - { - Variables[SuppressWritesKey] = true; - } + // Set unconditionally after Reset() (which doesn't touch this key): a WorkflowTree instance + // can be reused across executions, so the flag must reflect only the current call - never a + // value left over from a previous suppressWrites: true run, which would otherwise silently + // suppress a real workflow's Update. + Variables[SuppressWritesKey] = suppressWrites; Variables["InputEntities(\"primaryEntity\")"] = primaryEntity; Variables["ExecutionTime"] = DateTime.Now.Add(timeOffset); var transactioncurrencyid = "transactioncurrencyid";