Fix: calculated-field evaluation must not persist via a spurious Update (#340)

* Fix: calculated-field evaluation must not persist via a spurious Update

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 <noreply@anthropic.com> via Conducktor <conducktor@contextand.com>

Author: mkholt

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)
 

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<ITracingService>());
+                    factory, factory.GetService<ITracingService>(), suppressWrites: true);
 
                 if (resultTree.Variables.TryGetValue("InputEntities(\"primaryEntity\")", out var resultObj)
                     && resultObj is Entity result && result.Contains(attr.LogicalName))

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<EntityReference>("transactioncurrencyid"))
-                    .GetAttributeValue<string>("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<EntityReference>("transactioncurrencyid");
+                if (currencyRef == null)
+                {
+                    return null;
+                }
 
+                var currencysymbol = db.GetEntity(currencyRef).GetAttributeValue<string>("currencysymbol");
                 return currencysymbol + (value as Money).Value.ToString();
             }
 

src/XrmMockup365/Workflow/WorkflowNode/SetAttributeValue.cs

@@ -36,6 +36,16 @@ public void Execute(ref Dictionary<string, object> 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);
 

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 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";

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<DateTime>("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()
         {