From 9ead93a2a0b3137cb0da48f19f37295ba18280b8 Mon Sep 17 00:00:00 2001 From: Claire Kuang Date: Fri, 9 Sep 2022 15:27:27 +0100 Subject: [PATCH] hotfix(rhino): handles commits with objects with duplicate ids (#1604) handles commits with objects with duplicate ids --- .../UI/ConnectorBindingsRhino.cs | 145 ++++++++++++++---- .../ConnectorRhinoShared/Utils.cs | 7 +- 2 files changed, 116 insertions(+), 36 deletions(-) diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/ConnectorBindingsRhino.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/ConnectorBindingsRhino.cs index 0a347f8224..4f6c6b5aa9 100644 --- a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/ConnectorBindingsRhino.cs +++ b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/ConnectorBindingsRhino.cs @@ -268,7 +268,7 @@ public override async Task PreviewReceive(StreamState state, Progre int count = 0; var commitLayerName = DesktopUI2.Formatting.CommitInfo(state.CachedStream.name, state.BranchName, commit.id); // get commit layer name - Preview = FlattenCommitObject(commitObject, converter, commitLayerName, ref count); + Preview = FlattenCommitObject(commitObject, converter, progress, commitLayerName, ref count); Doc.Notes += "%%%" + commitLayerName; // give converter a way to access commit layer info // Convert preview objects @@ -284,18 +284,27 @@ public override async Task PreviewReceive(StreamState state, Progre progress.Report.Log(previewObj); continue; } - previewObj.Converted = previewObj.Convertible ? ConvertObject(previewObj, converter) : previewObj.Fallback.SelectMany(o => ConvertObject(o, converter)).ToList(); + + if (previewObj.Convertible) + previewObj.Converted = ConvertObject(previewObj, converter); + else + foreach (var fallback in previewObj.Fallback) + { + fallback.Converted = ConvertObject(fallback, converter); + previewObj.Log.AddRange(fallback.Log); + } if (previewObj.Converted == null || previewObj.Converted.Count == 0) { - previewObj.Update(status: ApplicationObject.State.Failed, logItem: $"Couldn't convert object or any fallback values"); + var convertedFallback = previewObj.Fallback.Where(o => o.Converted != null || o.Converted.Count > 0); + if (convertedFallback != null && convertedFallback.Count() > 0) + previewObj.Update(status: ApplicationObject.State.Created, logItem: $"Creating with {convertedFallback.Count()} fallback values"); + else + previewObj.Update(status: ApplicationObject.State.Failed, logItem: $"Couldn't convert object or any fallback values"); } else - { previewObj.Status = ApplicationObject.State.Created; - if (!previewObj.Convertible) - previewObj.Update(logItem: $"Created using {previewObj.Converted.Count} fallback values"); - } + progress.Report.Log(previewObj); } progress.Report.Merge(converter.Report); @@ -311,7 +320,16 @@ public override async Task PreviewReceive(StreamState state, Progre } // create display conduit - PreviewConduit = new PreviewConduit(Preview); + try + { + PreviewConduit = new PreviewConduit(Preview); + } + catch(Exception e) + { + progress.Report.OperationErrors.Add(new Exception($"Could not create preview: {e.Message}")); + ResetDocument(); + return null; + } PreviewConduit.Enabled = true; Doc.Views.ActiveView.ActiveViewport.ZoomBoundingBox(PreviewConduit.bbox); Doc.Views.Redraw(); @@ -367,18 +385,28 @@ public override async Task ReceiveStream(StreamState state, Progres { // flatten the commit object to retrieve children objs int count = 0; - Preview = FlattenCommitObject(commitObject, converter, commitLayerName, ref count); + Preview = FlattenCommitObject(commitObject, converter, progress, commitLayerName, ref count); // convert foreach (var previewObj in Preview) { - previewObj.Converted = previewObj.Convertible ? ConvertObject(previewObj, converter) : previewObj.Fallback.SelectMany(o => ConvertObject(o, converter)).ToList(); - - if (previewObj.Converted == null || previewObj.Converted.Count == 0) - previewObj.Update(status: ApplicationObject.State.Failed, logItem: $"Couldn't convert object or any fallback values"); + if (previewObj.Convertible) + previewObj.Converted = ConvertObject(previewObj, converter); else - if (!previewObj.Convertible) - previewObj.Update(logItem: $"Creating with {previewObj.Converted.Count} fallback values"); + foreach (var fallback in previewObj.Fallback) + { + fallback.Converted = ConvertObject(fallback, converter); + previewObj.Log.AddRange(fallback.Log); + } + + if (previewObj.Converted == null || previewObj.Converted.Count == 0) + { + var convertedFallback = previewObj.Fallback.Where(o => o.Converted != null || o.Converted.Count > 0); + if (convertedFallback != null && convertedFallback.Count() > 0) + previewObj.Update(logItem: $"Creating with {convertedFallback.Count()} fallback values"); + else + previewObj.Update(status: ApplicationObject.State.Failed, logItem: $"Couldn't convert object or any fallback values"); + } progress.Report.Log(previewObj); if (progress.CancellationTokenSource.Token.IsCancellationRequested) @@ -394,7 +422,17 @@ public override async Task ReceiveStream(StreamState state, Progres { // bake previewObj.CreatedIds.Clear(); // clear created ids before bake because these may be speckle ids from the preview - BakeObject(previewObj, converter); + + if (previewObj.Convertible) + BakeObject(previewObj, converter); + else + { + foreach (var fallback in previewObj.Fallback) + BakeObject(fallback, converter, previewObj); + previewObj.Status = previewObj.Fallback.Where(o => o.Status == ApplicationObject.State.Failed).Count() == previewObj.Fallback.Count ? + ApplicationObject.State.Failed : ApplicationObject.State.Created; + } + progress.Report.Log(previewObj); if (progress.CancellationTokenSource.Token.IsCancellationRequested) @@ -457,19 +495,23 @@ private async Task GetCommit(Commit commit, StreamState state, ProgressVie } // Recurses through the commit object and flattens it. Returns list of Preview objects - private List FlattenCommitObject(object obj, ISpeckleConverter converter, string layer, ref int count, bool foundConvertibleMember = false) + private List FlattenCommitObject(object obj, ISpeckleConverter converter, ProgressViewModel progress, string layer, ref int count, bool foundConvertibleMember = false) { var objects = new List(); if (obj is Base @base) { - var speckleType = @base.speckle_type.Split(new char[] { ':' }, StringSplitOptions.RemoveEmptyEntries).FirstOrDefault(); + var speckleType = @base.speckle_type.Split(new char[] { ':' }, StringSplitOptions.RemoveEmptyEntries).LastOrDefault(); var appObj = new ApplicationObject(@base.id, speckleType) { applicationId = @base.applicationId, Container = layer }; if (converter.CanConvertToNative(@base)) { appObj.Convertible = true; + if (StoredObjects.ContainsKey(@base.id)) + appObj.Update(logItem: $"Found another {speckleType} in this commit with the same id. Skipped other object"); + else + StoredObjects.Add(@base.id, @base); + objects.Add(appObj); - StoredObjects.Add(@base.id, @base); return objects; } else @@ -481,7 +523,7 @@ private List FlattenCommitObject(object obj, ISpeckleConverte bool hasFallback = false; if (@base.GetMembers().ContainsKey("displayValue")) { - var fallbackObjects = FlattenCommitObject(@base["displayValue"], converter, layer, ref count, foundConvertibleMember); + var fallbackObjects = FlattenCommitObject(@base["displayValue"], converter, progress, layer, ref count, foundConvertibleMember); if (fallbackObjects.Count > 0) { appObj.Fallback.AddRange(fallbackObjects); @@ -491,8 +533,11 @@ private List FlattenCommitObject(object obj, ISpeckleConverte } if (hasFallback) { + if (StoredObjects.ContainsKey(@base.id)) + appObj.Update(logItem: $"Found another {speckleType} in this commit with the same id. Skipped other object"); + else + StoredObjects.Add(@base.id, @base); objects.Add(appObj); - StoredObjects.Add(@base.id, @base); } // handle any children elements, these are added as separate previewObjects @@ -508,7 +553,7 @@ private List FlattenCommitObject(object obj, ISpeckleConverte string objLayerName = prop.StartsWith("@") ? prop.Remove(0, 1) : prop; string rhLayerName = objLayerName.StartsWith($"{layer}{Layer.PathSeparator}") ? objLayerName : $"{layer}{Layer.PathSeparator}{objLayerName}"; - var nestedObjects = FlattenCommitObject(@base[prop], converter, rhLayerName, ref count, foundConvertibleMember); + var nestedObjects = FlattenCommitObject(@base[prop], converter, progress, rhLayerName, ref count, foundConvertibleMember); var validNestedObjects = nestedObjects.Where(o => o.Convertible == true || o.Fallback.Count > 0)?.ToList(); if (validNestedObjects != null && validNestedObjects.Count > 0) { @@ -531,7 +576,7 @@ private List FlattenCommitObject(object obj, ISpeckleConverte { count = 0; foreach (var listObj in list) - objects.AddRange(FlattenCommitObject(listObj, converter, layer, ref count)); + objects.AddRange(FlattenCommitObject(listObj, converter, progress, layer, ref count)); return objects; } @@ -539,7 +584,7 @@ private List FlattenCommitObject(object obj, ISpeckleConverte { count = 0; foreach (DictionaryEntry kvp in dict) - objects.AddRange(FlattenCommitObject(kvp.Value, converter, layer, ref count)); + objects.AddRange(FlattenCommitObject(kvp.Value, converter, progress, layer, ref count)); return objects; } @@ -569,7 +614,8 @@ void FlattenConvertedObject(object item) return convertedList; } - private void BakeObject(ApplicationObject previewObj, ISpeckleConverter converter) + + private void BakeObject(ApplicationObject previewObj, ISpeckleConverter converter, ApplicationObject parent = null) { var obj = StoredObjects[previewObj.OriginalId]; int bakedCount = 0; @@ -586,23 +632,35 @@ private void BakeObject(ApplicationObject previewObj, ISpeckleConverter converte string layerPath = previewObj.Container; if (!o.IsValidWithLog(out string log)) { - previewObj.Update(logItem: $"{log.Replace("\n", "").Replace("\r", "")}"); + var invalidMessage = $"{log.Replace("\n", "").Replace("\r", "")}"; + if (parent != null) + parent.Update(logItem: $"fallback {previewObj.id}: {invalidMessage}"); + else + previewObj.Update(logItem: invalidMessage); continue; } Layer bakeLayer = Doc.GetLayer(layerPath, true); if (bakeLayer == null) { - previewObj.Update(logItem: $"Could not create layer {layerPath}."); + var layerMessage = $"Could not create layer {layerPath}."; + if (parent != null) + parent.Update(logItem: $"fallback {previewObj.id}: {layerMessage}"); + else + previewObj.Update(logItem: layerMessage); continue; } var attributes = new ObjectAttributes(); // handle display style if (obj[@"displayStyle"] is Base display) + { if (converter.ConvertToNative(display) is ObjectAttributes displayAttribute) attributes = displayAttribute; - else if (obj[@"renderMaterial"] is Base renderMaterial) - attributes.ColorSource = ObjectColorSource.ColorFromMaterial; + } + else if (obj[@"renderMaterial"] is Base renderMaterial) + { + attributes.ColorSource = ObjectColorSource.ColorFromMaterial; + } // assign layer attributes.LayerIndex = bakeLayer.Index; @@ -613,10 +671,18 @@ private void BakeObject(ApplicationObject previewObj, ISpeckleConverter converte Guid id = Doc.Objects.Add(o, attributes); if (id == Guid.Empty) { - previewObj.Update(logItem: $"Could not add to document."); + var bakeMessage = $"Could not bake to document."; + if (parent != null) + parent.Update(logItem: $"fallback {previewObj.id}: {bakeMessage}"); + else + previewObj.Update(logItem: bakeMessage); continue; } - previewObj.Update(createdId: id.ToString()); + + if (parent != null) + parent.Update(createdId: id.ToString()); + else + previewObj.Update(createdId: id.ToString()); bakedCount++; @@ -633,11 +699,17 @@ private void BakeObject(ApplicationObject previewObj, ISpeckleConverter converte } break; case RhinoObject o: // this was prbly a block instance, baked during conversion - previewObj.Update(status: ApplicationObject.State.Created, createdId: o.Id.ToString()); + if (parent != null) + parent.Update(createdId: o.Id.ToString()); + else + previewObj.Update(status: ApplicationObject.State.Created, createdId: o.Id.ToString()); bakedCount++; break; case string o: // this was prbly a view, baked during conversion - previewObj.Update(status: ApplicationObject.State.Created, createdId: o); + if (parent != null) + parent.Update(createdId: o); + else + previewObj.Update(status: ApplicationObject.State.Created, createdId: o); bakedCount++; break; default: @@ -646,7 +718,12 @@ private void BakeObject(ApplicationObject previewObj, ISpeckleConverter converte } if (bakedCount == 0) - previewObj.Update(status: ApplicationObject.State.Failed, logItem: $"Could not bake object"); + { + if (parent != null) + parent.Update(logItem: $"fallback {previewObj.id}: could not bake object"); + else + previewObj.Update(status: ApplicationObject.State.Failed, logItem: $"Could not bake object"); + } else previewObj.Update(status: ApplicationObject.State.Created); } diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Utils.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Utils.cs index 1e71bd3a92..548a0f3d42 100644 --- a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Utils.cs +++ b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Utils.cs @@ -99,7 +99,8 @@ public PreviewConduit(List preview) foreach (var previewObj in preview) { var converted = new List(); - foreach (var obj in previewObj.Converted) + var toBeConverted = previewObj.Convertible ? previewObj.Converted : previewObj.Fallback.SelectMany(o => o.Converted).ToList(); + foreach (var obj in toBeConverted) { switch (obj) { @@ -118,7 +119,9 @@ public PreviewConduit(List preview) } converted.Add(obj); } - Preview.Add(previewObj.OriginalId, converted); + + if (!Preview.ContainsKey(previewObj.OriginalId)) + Preview.Add(previewObj.OriginalId, converted); } }